Skip to content

[v2] Add versioned endpoint discovery#3435

Merged
mandre merged 16 commits intogophercloud:v2from
shiftstack:endpoint-discovery-v2
Aug 18, 2025
Merged

[v2] Add versioned endpoint discovery#3435
mandre merged 16 commits intogophercloud:v2from
shiftstack:endpoint-discovery-v2

Conversation

@stephenfin
Copy link
Copy Markdown
Contributor

This is a manual backport of #3351 and #3420, with #3108 also included to avoid tricky merge conflicts. A backport of #3108 is proposed separately at #3434 to ease reviews. We should probably merge that first and then come back to rebase and merge this.

@github-actions github-actions bot added edit:networking This PR updates networking code edit:openstack This PR updates common OpenStack code edit:dns This PR updates dns code edit:common This PR updates common code edit:actions This PR updates GitHub Actions code edit:image This PR updates image code edit:orchestration This PR updates orchestration code edit:db This PR updates db code edit:loadbalancer This PR updates loadbalancer code edit:compute This PR updates compute code edit:keymanager This PR updates keymanager code edit:utils This PR updates utils code v2 This PR targets v2 edit:messaging This PR updates messaging code edit:containerinfra This PR updates containerinfra code edit:placement This PR updates placement code edit:objectstorage This PR updates objectstorage code edit:workflow This PR updates workflow code edit:sharedfilesystems This PR updates sharedfilesystems code edit:baremetal This PR updates baremetal code edit:gophercloud This PR updates common Gophercloud code edit:blockstorage This PR updates blockstorage code edit:identity This PR updates identity code edit:container This PR updates container code edit:baremetalintrospection This PR updates baremetalintrospection code edit:testinfra This PR updates testing infrastructure code semver:major Breaking change labels Jun 19, 2025
@kayrus
Copy link
Copy Markdown
Contributor

kayrus commented Jul 15, 2025

we hit this issue in terraform-provider-openstack/terraform-provider-openstack#1929
any objections on merging this? this will also require #3442 backport

kayrus
kayrus previously approved these changes Jul 15, 2025
@stephenfin
Copy link
Copy Markdown
Contributor Author

we hit this issue in terraform-provider-openstack/terraform-provider-openstack#1929 any objections on merging this? this will also require #3442 backport

Not from me. I'm happy to merge this now. It's been stable on main.

We do not need to check the validity of the provided opts more than once
so don't. We can also simplify our handling of multiple endpoints
(though not as much as we'd like in the v2 case, due to forthcoming
patches).

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 4329495)
We will use this to do proper discovery across multiple versions.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit bae4c6f)
This retrieves the base endpoint without stripping the version information.
This is helpful for service catalog entries that include project IDs, like
those historically preferred by Cinder for some reason.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 2bf97ce)
This is an re-implementation of GetSupportedMicroversions that supports
use with a ProviderClient and an explicit endpoint URL, allowing us to
use this during discovery.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 8cbcb2f)
For now these are exact duplicates of the V2EndpointURL and V3EndpointURL
helpers but with different names.

  diff -u openstack/endpoint_location.go openstack/endpoint.go

This is done to ease review. We will introduce further differences shortly.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit ed23d9b)
This is simpler than expected. Effectively, for each potential endpoint,
we retrieve the version document from the root URL and parse the version
information, if any, from it, comparing it against the version expected.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit f28c963)
Avoid unnecessary lookups for when we are encoding the version in our
service type.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 07d1ee0)
And deprecate the old ones since they are no longer used.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 8139f99)
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 82c2ca0)
Ahead of the addition of some new tests.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 774a8b8)
Right now these are effectively duplicates of the tests for
GetSupportedMicroversions, which is also expanded here to test against
the version documents of other services, but that will change shortly.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 9c18abd)
In commit f28c963, we added support for
discovering API versions. To do this, we added a dependency on the
'version' and 'min_version' headers, which are used by services with
microversion support to document the maximum and minimum API versions
supported, respectively. However, not all services support API
microversions: services like Glance and Designate use API versions as a
signal of a new feature (or, in Glance's case, as a signal that a
feature is not enabled), others like Keystone just support a single
version, while Neutron does its own thing with API extensions [1]. Given
this fact, relying on these fields is a mistake.

Instead, we should be relying on the 'id' field. Per the api-sig
guidelines [2], this should be the API major version [3]. We continue
parsing the microversion-related headers, since it will be useful later
on if/when we want to do versioned discovery.

[1] https://that.guru/blog/api-versioning-in-openstack/
[2] https://specs.openstack.org/openstack/api-sig/guidelines/discoverability.html
[3] The studious among you may notice that the api-sig guidelines
    indicate that the maximum API microversion should be exposed via the
    'max_version' header. However, in practice, virtually everyone uses
    'version' instead. Why? Who knows. Best to just take these things on
    the chin and move on with our lives.
[4] The term "major version" is a bit loaded. Consider Nova: at the time
    of writing, it exposes two "major versions": v2.0 and v2.1. v2.0
    does not support microversions. v2.1 does. For v2.1 you therefore
    also have microversions to content with and at the time of writing
    it supports a minimum microversion of 2.1 and a maximum microversion
    of 2.100 (no 'v' prefix here). Normalizing these as we've done here
    gives us a major version of 2, a minor version of 1, a major maximum
    microversion of 2, a minor maximum microversion of 100, a major
    minimum microversion of 2, and a minor minimum microversion of 1 🤯.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 5b89285)
While the expected format of these documents is defined by a spec [1],
most services do things slightly differently. Here, we add support for
the Keystone and Barbican-style documents, which embed version objects
inside another object with a single 'values' key, like so:

  {"versions": {"values": [{"id": "v3.14", ...}]}}

And the Magnum-style document, which doesn't envelope individual version
objects at all:

  {"id": "v2", ...}

These are in contrast to the format used by Nova, Cinder etc.:

  {"versions": [{"id": "v2", ...}]}

We also add support for 'max_version' key, which is used by Magnum and
is what the spec actually recommends.

[1] https://specs.openstack.org/openstack/api-sig/guidelines/discoverability.html

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 6e92c22)
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit 37ec207)
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
(cherry picked from commit b6be984)
@stephenfin stephenfin force-pushed the endpoint-discovery-v2 branch from a3efa14 to a7234bb Compare August 6, 2025 16:18
@stephenfin
Copy link
Copy Markdown
Contributor Author

@stephenfin can you add #3442 into this PR and make a rebase one more time?

I rebased, but can we add #3442 in here? That looks like a breaking change?

regarding the redundant API calls, this is what I mean, see a terraform example:

data "openstack_networking_port_v2" "port_1" {
  count= 100
  name = "port_1"
}

dev provider compiled with gophercloud with version discovery:

$ TF_LOG=DEBUG terraform apply 2>&1 | grep 'GET ' | wc -l
200

recent release provider:

$ TF_LOG=DEBUG terraform apply 2>&1 | grep 'GET ' | wc -l
100

UPD: if you still insist on discovery, then we need to reopen a #1987 to have a way to create a client only once and clone it for each terraform resource (to properly support microversion)

I don't see any way to avoid discovery: it's what OpenStack insists on. However, yes, we should be caching the discovery documents. I can work on this.

@kayrus
Copy link
Copy Markdown
Contributor

kayrus commented Aug 6, 2025

That looks like a breaking change?

yes, without this change the discovery will be broken.

@stephenfin
Copy link
Copy Markdown
Contributor Author

That looks like a breaking change?

yes, without this change the discovery will be broken.

Indeed. I think we need an alternative option so. From the linked issue, the issue is here:

client.EndpointLocator = func(opts gophercloud.EndpointOpts) (string, error) {
return V3Endpoint(ctx, client, catalog, opts)
}

Could we hardcode context.TODO() instead of using a provided ctx argument (only on v2 of course)? It's not ideal, but I assume any timeouts will be handled elsewhere

This is a v2-only alternative to gophercloud#3442 [1] that avoids introducing a
breaking function signature change while still resolving gophercloud#3441 [2].

[1] gophercloud#3442
[2] gophercloud#3441

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@stephenfin
Copy link
Copy Markdown
Contributor Author

Could we hardcode context.TODO() instead of using a provided ctx argument (only on v2 of course)? It's not ideal, but I assume any timeouts will be handled elsewhere

I've done this.

@mandre
Copy link
Copy Markdown
Contributor

mandre commented Aug 14, 2025

@stephenfin is this PR ready to merge? I'd like to cut a new v2 release soon and wondering if this PR should be included.

@stephenfin
Copy link
Copy Markdown
Contributor Author

I think so, yes

@stephenfin stephenfin removed the hold Do not merge label Aug 15, 2025
@mandre
Copy link
Copy Markdown
Contributor

mandre commented Aug 18, 2025

OK, let's get it in then.

@mandre mandre merged commit 60e11d4 into gophercloud:v2 Aug 18, 2025
61 of 64 checks passed
@mandre mandre deleted the endpoint-discovery-v2 branch August 18, 2025 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

edit:actions This PR updates GitHub Actions code edit:baremetal This PR updates baremetal code edit:baremetalintrospection This PR updates baremetalintrospection code edit:blockstorage This PR updates blockstorage code edit:common This PR updates common code edit:compute This PR updates compute code edit:container This PR updates container code edit:containerinfra This PR updates containerinfra code edit:db This PR updates db code edit:dns This PR updates dns code edit:gophercloud This PR updates common Gophercloud code edit:identity This PR updates identity code edit:image This PR updates image code edit:keymanager This PR updates keymanager code edit:loadbalancer This PR updates loadbalancer code edit:messaging This PR updates messaging code edit:networking This PR updates networking code edit:objectstorage This PR updates objectstorage code edit:openstack This PR updates common OpenStack code edit:orchestration This PR updates orchestration code edit:placement This PR updates placement code edit:sharedfilesystems This PR updates sharedfilesystems code edit:testinfra This PR updates testing infrastructure code edit:utils This PR updates utils code edit:workflow This PR updates workflow code semver:minor Backwards-compatible change v2 This PR targets v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants