Skip to content

Validate vNet creation requests before creating the network#1997

Merged
cb-github-robot merged 1 commit into
cloud-barista:mainfrom
yunkon-kim:250417-15
Apr 29, 2025
Merged

Validate vNet creation requests before creating the network#1997
cb-github-robot merged 1 commit into
cloud-barista:mainfrom
yunkon-kim:250417-15

Conversation

@yunkon-kim
Copy link
Copy Markdown
Member

This PR will validate vNet creation requests before creating the network.

For example, the TbVNetReq{} entered by the user is validated through ValidateVNetReq(reqt) as follows.

	// Create vNet
	// [Input] Bind the request body
	reqt := &model.TbVNetReq{}
	if err := c.Bind(reqt); err != nil {
		return c.JSON(http.StatusBadRequest, model.SimpleMsg{Message: err.Error()})
	}

	// [Validation] Validate the request
	err = resource.ValidateVNetReq(reqt)
	if err != nil {
		log.Error().Err(err).Msg("")
		return c.JSON(http.StatusBadRequest, model.SimpleMsg{Message: err.Error()})
	}

Note: Validation using TbVNetReq{} should be applied to the appropriate sections.

The information needed for validation is described in assets/networkinfo.yaml. It’s based on documentation and console/portal details from each CSP.

Each item has been carefully analyzed and compared. But please note that some parts might be inaccurate or become outdated over time — so contributions to help keep it accurate and up-to-date are always welcome!


It was tested by /ns/{nsId}//resources/vNet API

  • nsId: default
{
  "cidrBlock": "10.0.0.0/16",
  "connectionName": "aws-ap-northeast-2",
  "description": "vnet00 managed by CB-Tumblebug",
  "name": "vnet00",
  "subnetInfoList": [
    {
      "description": "subnet00 managed by CB-Tumblebug",
      "ipv4_CIDR": "10.0.1.0/24",
      "name": "subnet00",
      "zone": "ap-northeast-2a"
    }
  ]
}

(logs of the validation part)

cb-tumblebug       | 12:41PM DBG src/core/resource/vnet.go:131 > ValidateVNetReq
cb-tumblebug       | 12:41PM DBG src/core/resource/vnet.go:132 > vNetReq: &{Name:vnet00 ConnectionName:aws-ap-northeast-2 CidrBlock:10.0.0.0/16 SubnetInfoList:[{Name:subnet00 IPv4_CIDR:10.0.1.0/24 Zone:ap-northeast-2a Description:subnet00 managed by CB-Tumblebug}] Description:vnet00 managed by CB-Tumblebug}
cb-tumblebug       | 12:41PM DBG src/core/resource/vnet.go:305 > [Network Validation Success] vNet CIDR block 10.0.0.0/16 is available for use in the CSP (provider: aws)
cb-tumblebug       | 12:41PM DBG src/core/resource/vnet.go:317 > [Network Validation Success] vNet CIDR block 10.0.0.0/16 is valid (provider: aws, prefix min: 16, prefix max: 28)
cb-tumblebug       | 12:41PM DBG src/core/resource/vnet.go:344 > [Network Validation Success] vNet CIDR block 10.0.0.0/16 is not in the reserved CIDR blocks (provider: aws)
cb-tumblebug       | 12:41PM DBG src/core/resource/vnet.go:382 > [Network Validation Success] subnet CIDR block 10.0.0.0/16 is valid (provider: aws, prefix min: 16, prefix max: 28)
cb-tumblebug       | 12:41PM INF src/core/resource/vnet.go:383 > [Network Validation Completed] Everything is valid (provider: aws)
cb-tumblebug       | 12:41PM DBG src/core/resource/vnet.go:227 > network: {CidrBlock:10.0.0.0/16 Name: Subnets:[{CidrBlock:10.0.1.0/24 Name: Subnets:[]}]}
cb-tumblebug       | 12:41PM INF src/core/resource/vnet.go:493 > CreateVNet

@yunkon-kim yunkon-kim requested review from Copilot and removed request for seokho-son and sykim-etri April 22, 2025 13:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces validations for vNet creation requests to ensure that CIDR blocks, subnet configurations, and related parameters comply with provider-specific rules before provisioning.

  • Added loading of network configuration from a new networkinfo YAML file in setConfig.
  • Enhanced vNet request validation in ValidateVNetReq with detailed checks for CIDR block availability and prefix ranges via IsAvailableForUseInCSP.
  • Introduced new network configuration models in config.go to support these validations.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

File Description
src/main.go Added networkinfo loading logic and JSON debug output for network configuration.
src/core/resource/vnet.go Extended vNet request validation and implemented detailed CIDR and subnet checks.
src/core/model/config.go Introduced new network configuration structures to support vNet validations.
src/core/common/config.go Global variable for network configuration added for runtime usage.

Comment thread src/main.go Outdated
Comment thread src/core/resource/vnet.go Outdated
Comment thread src/core/resource/vnet.go Outdated
Comment thread assets/networkinfo.yaml Outdated
reserved-ips:
value: 4
description: Number of reserved IPs in the subnet (i.e., the 1st IP address and last 3 IP addresses are reserved)
region:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yunkon-kim region 부분은 cloudinfo 와 중복 내용일 것 같은데, 추가하신 의도가 있으신지요?

Copy link
Copy Markdown
Member Author

@yunkon-kim yunkon-kim Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seokho-son

networkinfo.yaml은 CSP의 네트워크 특성/요구사항 정보를 관리하기 위한 목적으로 만들었습니다. 네트워크와 관련된 정보라면 중복되는 정보가 있더라도 기재해 두고자 하였습니다.

한편으로는

  1. 기존 정보와 중복되는 부분을 바로 파악할 수 있어 cloudinfo와 networkinfo 병합에 대해 고려해 볼 때 이점이 있을 것이고,
  2. 병합이 되지 않아도 각각 독립적으로 적용할 수 있겠다고 생각했습니다.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yunkon-kim 목적 공유 감사합니다.

중복되는 내용 자체가 상당히 많고 시스템 내에서 변수로 쉽게 접근 되는 정보라서, 최종 버전이라고 한다면 내용을 조정하는 것이 좋을 것 같고 (관리 포인트를 줄이기 위해서 입니다. 참고로, cloudinfo는 현재 내용을 자동 생성하는 별도의 도구가 있습니다.),
말씀하신 것과 같이 병합 및 조정될 것을 감안하고 임시로 활용하시는 것이라면, 현 상태로 승인해도 무방할 것 같지만, 당장에 활용도가 크지 않다면, 정리하는 것도 좋을 것 같습니다.

추가로, cloudinfo에는 현재 클라우드 정보 등록에 필요한 정보들로만 구성되어 있습니다. (csps, drivers, regions,..) 해당 구성에 리소스(ex: vm, disk, net, ...)의 csp별 상세 설정(및 제한 범위) 등이 포함되어 있지는 않습니다.

그래서, 정리를 하자면, cloudinfo 는 기존 용도대로 유지(단 명칭 변경이 필요할지는 고민 필요. cloudinfo --> cloudregisoninfo)하고,
리소스별로 제한해야 하는 추가 정보가 있다면, networkinfo.yaml , https://github.com/cloud-barista/cb-tumblebug/blob/main/assets/k8sclusterinfo.yaml 과 같이 리소스별 별도의 파일을 구성하여 처리하면 어떨까 싶습니다. (가급적 정보는 중복되지 않도록 하고요)

사실.. 조정이 필요한 파일은
https://github.com/cloud-barista/cb-tumblebug/blob/main/conf/cloud_conf.yaml
일거라고 생각합니다. ;)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seokho-son 자세히 설명해 주셔서 감사합니다!

설명을 듣고 보니 cloudinfo와 중복되는 부분이 없도록 networkinfo를 수정하는 것이 좋을 것 같습니다. 네트워크 검증할 때 cloudinfo의 zone 정보를 참조하는 방향으로 수정을 진행하겠습니다.

제가 https://github.com/cloud-barista/cb-tumblebug/blob/main/conf/cloud_conf.yaml 조정에 대해서 캐치하지 못하였습니다;;; 이번 PR과 관련이 있다면 어떤 측면의 조정을 생각하고 계신지 설명 부탁 드립니다 :)

Copy link
Copy Markdown
Member

@seokho-son seokho-son Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yunkon-kim

conf에 들어가 있는 내용은 단순 info와 다르게, 사용자가 필요에 따라 설정할 수 있는 항목이라고 생각하고 만들었으나conf/cloud_conf.yaml

assets/*info.yaml 쪽과 의미와 용도가 중복되거나, 구분하기 까다로운 경우들이 발생하고 있는 것 같아서, 이에 대한 검토 및 조정이 필요할 것으로 보고 있었습니다. ^^

현재는 파일로 관리되고 있는 정보들이 아래와 같은데, 완전 구분하기에는 까다롭네요. :)

  1. 단순히 클라우드 관련 정보를 시스템에 주입하기 위한 정보
  2. cloud-barista의 제약에 의한 사항을 명시하기 위한 정보
  3. 사용자 편의를 위해 임의 지정된 항목 (사용자 자율 변경 가능) 등

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seokho-son 그렇네요. 완전한 구분이 어려울 것 같습니다.

대략적으로 보자면 cloud_conf에 2번에 해당하는 제약에 의한 사항들이 기재되어 있는 것 같고요. *info.yaml에는 1번(3번도 조금은 포함된 것 같기도 합니다)에 해당하는 사항들이 기록되어 있는 것 같습니다.

이와 관련해서는, 진행하면서 유의미한 방향을 잡아야 할 것 같습니다. 적절히 한 파일로 통합하거나, 지금처럼 독립된 형태로 유지하는 것 중 어느 것이 좋은지 살펴보도록 하겠습니다.

Copy link
Copy Markdown
Member

@seokho-son seokho-son left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi

Comment thread assets/networkinfo.yaml Outdated
Comment thread assets/networkinfo.yaml Outdated
* Describe network characteristics/requirements in `networkinfo.yaml`
  - Note: The info does not overlap with `cloudinfo`.
* Check and improve the validation criteria based on the information displayed in the console
* Update vNet and subnet validation functions
* Test this via the vNet creation API
@yunkon-kim
Copy link
Copy Markdown
Member Author

@seokho-son

지금까지 코멘트 주신 내용을 반영 후, Squash and force-push 처리했습니다.

@seokho-son
Copy link
Copy Markdown
Member

/approve

@github-actions github-actions Bot added the approved This PR is approved and will be merged soon. label Apr 29, 2025
@cb-github-robot cb-github-robot merged commit 804a594 into cloud-barista:main Apr 29, 2025
5 checks passed
@yunkon-kim yunkon-kim deleted the 250417-15 branch April 29, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved This PR is approved and will be merged soon. asset src

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants