Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NCP,KT,NHN] use strings.EqualFold to check strings (region, zone, csp name) #1182

Closed
seokho-son opened this issue May 2, 2024 · 9 comments · Fixed by cloud-barista/cb-tumblebug#1567
Assignees
Labels
enhancement New feature or request

Comments

@seokho-son
Copy link
Member

What would you like to be enhanced
[NCP,KT,NHN]의 경우, 리전 명칭 확인을 위해서 string match 를 하는 것 같습니다.
이때, 비교 방식을 대소문자 구분하지 않도록 처리 요청드립니다.
(ProviderName, RegionName, Region, Zone 등)

예를 들어, NCPVPC의 경우
리전 스트링을 대문자만 허용하는 것으로 보입니다. (kr 로 요청시 오류 리턴)
"Not found region. Available Region : [KR, SGN, JPN]"

Why is this needed
대소문자를 구분함과 동시에 CSP마다 다른 대소문자 형태를 띄면, 사용자 사용 일관성을 저해하게 됩니다.
(CB-TB 에서는 가급적 strings.EqualFold 를 활용하거나, 모두 소문자로 치환하여 비교하고 있습니다.)

특별히 대소문자 구분에 의미가 있지 않다면
사용자 사용 일관성 향상을 위해서 개선을 요청 드립니다.

Proposed solution
비교 구문에 strings.EqualFold(x,y) 함수를 활용하시면 됩니다.

@seokho-son seokho-son added the enhancement New feature or request label May 2, 2024
@seokho-son
Copy link
Member Author

CB-Spider 전체적으로 관련이 되어 있을 수 있습니다. 이 경우 @powerkimhub 께 개선을 요청 드립니다.

드라이버 이슈라면, @innodreamer 께 개선을 요청 드립니다.

@seokho-son
Copy link
Member Author

혹시, 리전 조회시, KR, SGN, JPN 등 리전/존 명칭이 대문자로 CSP에서 넘어오는 것이 아니라, 대문자로 변환(또는 고정 문자를 대문자로 지정)해서 리턴해서 주시는 상황이라면, 이를 다른 글로벌 CSP들처럼 소문자로 변경 유지해주시는 것을 먼저 진행해주셔도 좋을 것 같습니다.

@innodreamer
Copy link
Member

innodreamer commented May 14, 2024

@seokho-son 먼저 말씀드리자면, CSP에서 region/zone 정보를 조회해서 제시해줄때 일부러 대문자로 변환해서 제시하지는 않고 있습니다.
꼭 소문자로 통일한다면 request call을 할때도 드라이버 내에서 대문자로 변경해서 call을 할 수 있지만, 위의 내용과 같은 상황에서 조회시에도 꼭 소문자로 보이도록 해야한다면 조회 시 드라이버 내에서 모두 소문자로 변환해서 제시되도록 해야할 거 같습니다.

아래의 사항에 대해 의견 주시기 바랍니다. @seokho-son @powerkimhub

  • 드라이버 내에서 CSP API request시 region/zone 이름을 대문자로 해야하는 경우, 소문자를 대문자로 변경해서 call 해야함.
  • 드라이버에서 전체적으로 region/zone 정보를 제시할 때 대문자는 소문자로 변경해서 제시해야함.

@seokho-son
Copy link
Member Author

@innodreamer

  • CSP API 상의 region/zone 리턴값이, 대문자인 경우가 있다는 말씀으로 이해하였습니다.
  • CSP API request 시에, 소문자로 요청시에 오류가 발생하는지 우선 문의 드립니다. (CSP가 요청값에도 대소문자를 구분하여 제한하고 있는 경우라면, CSP를 준용해주는 것이 좋겠다는 생각입니다.)

@innodreamer
Copy link
Member

@seokho-son 예를들어, NCP VPC의 경우 아래와 같이 조회되고
image

Config에 region/zone을 소문자로 지정하여 API call을 하면 아래와 같은 오류가 발생합니다.

[CLOUD-BARISTA].[ERROR]: 2024-05-14 16:39:19 RegionZoneHandler.go:196, github.com/cloud-barista/cb-spider/cloud-control-manager/cloud-driver/drivers/ncpvpc/resources.(*NcpRegionZoneHandler).ListOrgZone() - Failed to Get ZoneList from NCP Cloud : %!(EXTRA *errors.errorString=Failed to Get ZoneList from NCP Cloud : Status: 400 Bad Request, Body: {
"responseError": {
"returnCode": "1404",
"returnMessage": "Not found region. Available Region : [KR, HK, SGN, JPN, USWN, DEN]"
}
})

@seokho-son
Copy link
Member Author

@innodreamer 해당 오류가 CSP가 주는 오류로 보면 되나요?

@innodreamer
Copy link
Member

@seokho-son 네. 'Status: 400 ~ ' 부터 CSP 오류 메시지입니다.

@seokho-son
Copy link
Member Author

확인 감사합니다. 대소문자를 구분하는 CSP의 의도는 모르겠으나, 일단 준용하여,

CB-TB에서 리전/존의 대소문자 구분하여 CB-SP에 내려보내도록 하겠습니다.
차기 CB-TB 릴리스에 반영 후, 해당 이슈는 제가 close 하겠습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants