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

Fix BareMetalV1 version #2486

Merged
merged 1 commit into from
Aug 25, 2023
Merged

Conversation

snigle
Copy link
Contributor

@snigle snigle commented Oct 7, 2022

Fixes #2485

There is no documentation to link as it's only a configuration on keystone.

@coveralls
Copy link

coveralls commented Oct 7, 2022

Coverage Status

coverage: 79.131% (-0.009%) from 79.14% when pulling a934f01 on ovh:v1.0.0-ovh-baremetal-fix into dd0d36f on gophercloud:master.

@@ -363,7 +363,11 @@ func initClientOpts(client *gophercloud.ProviderClient, eo gophercloud.EndpointO
// NewBareMetalV1 creates a ServiceClient that may be used with the v1
// bare metal package.
func NewBareMetalV1(client *gophercloud.ProviderClient, eo gophercloud.EndpointOpts) (*gophercloud.ServiceClient, error) {
return initClientOpts(client, eo, "baremetal")
sc, err := initClientOpts(client, eo, "baremetal")
if !strings.HasSuffix(sc.Endpoint, "v1/") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that we have suffix v1 without the /?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes you are right maybe we nee double check v1/ and v1 ?

Copy link
Contributor

@reenjii reenjii Oct 7, 2022

Choose a reason for hiding this comment

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

Maybe it is best to check both ?

if !strings.HasSuffix(strings.TrimSuffix(sc.Endpoint, "/"), "v1")

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the initialization for the other clients, it seems unlikely the suffix has v1 without the /, but better be safe than sorry.

@EmilienM EmilienM added the semver:patch No API change label Aug 25, 2023
@EmilienM EmilienM merged commit c654d07 into gophercloud:master Aug 25, 2023
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch No API change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support of baremetal endpoint without /v1 in keystone catalog
6 participants