Skip to content

Authenticate with a clouds.yaml#2892

Merged
EmilienM merged 1 commit intogophercloud:masterfrom
pierreprinetti:auth_clouds
Feb 7, 2024
Merged

Authenticate with a clouds.yaml#2892
EmilienM merged 1 commit intogophercloud:masterfrom
pierreprinetti:auth_clouds

Conversation

@pierreprinetti
Copy link
Copy Markdown
Member

@pierreprinetti pierreprinetti commented Feb 5, 2024

This commit imports the clouds.yaml parsing code from the utils module, and exposes it in a way that fits the natural Gophercloud authentication flow.

Unlike the code from utils, this new clouds.Parse function keeps the separation between the ProviderClient (holding the cloud coordinates and a Keystone token) and the ServiceClient (holding specific endpoint configuration).

By default, clouds.Parse fetches its configuration from the environment, just like the openstack client would do.

Example use:

func main() {
	ctx := context.Background()
	ao, eo, tlsConfig, err := clouds.Parse()
	if err != nil {
		panic(err)
	}

	providerClient, err := config.NewProviderClient(ctx, ao, config.WithTLSConfig(tlsConfig))
	if err != nil {
		panic(err)
	}

	networkClient, err := openstack.NewNetworkV2(providerClient, eo)
	if err != nil {
		panic(err)
	}
}

The clouds.Parse function accepts several functional options that can modify its behaviour. For example, to use a clouds.yaml that exists in a non-standard path:

ao, eo, tlsConfig, err := clouds.Parse(clouds.WithLocations("/my/path/clouds2.yaml"))

It is also possible to pass a reader directly. Note that any number of options can be passed, with each of them taking precedence of the previous if there is conflict.

const exampleClouds = `clouds:
  openstack:
    auth:
      auth_url: https://example.com:13000`

ao, eo, tlsConfig, err := clouds.Parse(
	clouds.WithCloudsYAML(strings.NewReader(exampleClouds)),
	clouds.WithIdentityEndpoint("https://example.com:13001"),
	clouds.WithCloudName("osp1"),
	clouds.WithUsername("alice"),
)

@github-actions github-actions bot added semver:minor Backwards-compatible change and removed semver:minor Backwards-compatible change labels Feb 5, 2024
@coveralls
Copy link
Copy Markdown

coveralls commented Feb 5, 2024

Coverage Status

coverage: 77.23% (-0.7%) from 77.882%
when pulling d7e07ae on pierreprinetti:auth_clouds
into f17fe29 on gophercloud:master.

@github-actions github-actions bot added semver:minor Backwards-compatible change and removed semver:minor Backwards-compatible change labels Feb 5, 2024
@pierreprinetti pierreprinetti marked this pull request as ready for review February 5, 2024 13:19
Copy link
Copy Markdown
Contributor

@EmilienM EmilienM left a comment

Choose a reason for hiding this comment

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

I haven't spot any problems with it, although I haven't tested it yet. Before approving, I'll give it a try. In the meantime, I was wondering if you could have a least one e2e test for this one?

@pierreprinetti pierreprinetti requested a review from a team February 5, 2024 15:16
@github-actions github-actions bot added semver:minor Backwards-compatible change and removed semver:minor Backwards-compatible change labels Feb 6, 2024
@pierreprinetti
Copy link
Copy Markdown
Member Author

Replaced generic code with typed code in the function coalesce, as we currently only use it for strings. Generic code was preventing us from backporting this patch to v1.

@github-actions github-actions bot added semver:minor Backwards-compatible change and removed semver:minor Backwards-compatible change labels Feb 6, 2024
@pierreprinetti
Copy link
Copy Markdown
Member Author

Replaced a call to strings.CutPrefix with strings.TrimPrefix, which is compatible with Go v1.14.

@pierreprinetti
Copy link
Copy Markdown
Member Author

There may be more work required here, if e.g. we want ProviderClient and ServiceClient from v1 work with v2 (and vice-versa). This would require us to add getter methods to them (non-breaking change) and always accept an interfaces in the function calls (potentially breaking change).

@github-actions github-actions bot added semver:minor Backwards-compatible change and removed semver:minor Backwards-compatible change labels Feb 6, 2024
@EmilienM
Copy link
Copy Markdown
Contributor

EmilienM commented Feb 6, 2024

Finally managed to test this, really cool work!

~/gophercloud-playground via 🐹 v1.21.6 
❯ go run main.go
panic: cloud "" not found in clouds.yaml

goroutine 1 [running]:
main.main()
	/home/emilien/gophercloud-playground/main.go:20 +0x7f5
exit status 2

~/gophercloud-playground via 🐹 v1.21.6 
❯ export OS_CLOUD=foch_openshift

~/gophercloud-playground via 🐹 v1.21.6 on ☁️  foch_openshift(openshift) 
❯ go run main.go 

~/gophercloud-playground via 🐹 v1.21.6 on ☁️  foch_openshift(openshift) 
❯ openstack server list
+--------------------------------------+--------------+--------+----------------------------------------+--------------------------+-----------+
| ID                                   | Name         | Status | Networks                               | Image                    | Flavor    |
+--------------------------------------+--------------+--------+----------------------------------------+--------------------------+-----------+
| 17833e16-466d-4fbc-875a-cd36bc235f5f | test-emacchi | ACTIVE | hostonly=192.168.68.120, 2001:db8::221 | N/A (booted from volume) | m1.medium |
+--------------------------------------+--------------+--------+----------------------------------------+--------------------------+-----------+

@pierreprinetti
Copy link
Copy Markdown
Member Author

@EmilienM is that panic coming from your code, or from gophercloud?

@github-actions github-actions bot removed the semver:minor Backwards-compatible change label Feb 6, 2024
@EmilienM
Copy link
Copy Markdown
Contributor

EmilienM commented Feb 6, 2024

@EmilienM is that panic coming from your code, or from gophercloud?

gophercloud I think, my code is here: https://paste.centos.org/view/65f08709

it's because I didn't export OS_CLOUD.

@github-actions github-actions bot added the semver:minor Backwards-compatible change label Feb 6, 2024
@pierreprinetti
Copy link
Copy Markdown
Member Author

@EmilienM is that panic coming from your code, or from gophercloud?

gophercloud I think, my code is here: https://paste.centos.org/view/65f08709

it's because I didn't export OS_CLOUD.

line 20?

This commit imports the clouds.yaml parsing code from the utils module, and exposes it in a way that fits the natural Gophercloud authentication flow.

Unlike the code from utils, this new `clouds.Parse` function keeps the separation between the ProviderClient (holding the cloud coordinates and a Keystone token) and the ServiceClient (holding specific endpoint configuration).

By default, `clouds.Parse` fetches its configuration from the environment, just like the openstack client would do.

Example use:

```Go
func main() {
	ctx := context.Background()
	ao, eo, tlsConfig, err := clouds.Parse()
	if err != nil {
		panic(err)
	}

	providerClient, err := config.NewProviderClient(ctx, ao, config.WithTLSConfig(tlsConfig))
	if err != nil {
		panic(err)
	}

	networkClient, err := openstack.NewNetworkV2(providerClient, eo)
	if err != nil {
		panic(err)
	}
}
```

The `clouds.Parse` function accepts several functional options that can modify its behaviour. For example, to use a `clouds.yaml` that exists in a non-standard path:

```Go
ao, eo, tlsConfig, err := clouds.Parse(clouds.WithLocations("/my/path/clouds2.yaml"))
```

It is also possible to pass a reader directly. Note that any number of options can be passed, with each of them taking precedence of the previous if there is conflict.

```Go
const exampleClouds = `clouds:
  openstack:
    auth:
      auth_url: https://example.com:13000`

ao, eo, tlsConfig, err := clouds.Parse(
	clouds.WithCloudsYAML(strings.NewReader(exampleClouds)),
	clouds.WithIdentityEndpoint("https://example.com:13001"),
	clouds.WithCloudName("osp1"),
	clouds.WithUsername("alice"),
)
```
@github-actions github-actions bot added semver:minor Backwards-compatible change and removed semver:minor Backwards-compatible change labels Feb 6, 2024
@EmilienM
Copy link
Copy Markdown
Contributor

EmilienM commented Feb 6, 2024

There may be more work required here, if e.g. we want ProviderClient and ServiceClient from v1 work with v2 (and vice-versa). This would require us to add getter methods to them (non-breaking change) and always accept an interfaces in the function calls (potentially breaking change).

I think this could be done later, right? I'm going to approve this PR for now.

Copy link
Copy Markdown
Contributor

@mandre mandre left a comment

Choose a reason for hiding this comment

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

This is really cool 😎
LGTM

@EmilienM EmilienM merged commit dffc9f0 into gophercloud:master Feb 7, 2024
@pierreprinetti pierreprinetti deleted the auth_clouds branch February 7, 2024 22:56
@pierreprinetti pierreprinetti added the backport-v1 This PR will be backported to v1 label Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-v1 This PR will be backported to v1 semver:minor Backwards-compatible change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants