Repository navigation
Appcred auth support - #109
Merged
Merged
Conversation
Add AuthApplicationCredentialV3, a new AuthMode alongside the existing legacy/userpass/userpass-v3 modes, backed by a new V3AppCred Authenticator (identity/v3appcred.go). Goose previously only supported username+password auth (v2/v3) and EC2-style access/secret key pairs; there was no way to authenticate with an OpenStack application credential (id+secret), a distinct Keystone auth method with its own request shape and no explicit project scope (the credential is already scoped at creation time; Keystone rejects an explicit scope alongside it). Credentials gains ApplicationCredentialID/ApplicationCredentialSecret fields, and CredentialsFromEnv reads them from the standard OS_APPLICATION_CREDENTIAL_ID/OS_APPLICATION_CREDENTIAL_SECRET env vars, consistent with the existing OS_* variables it already reads. Adds a fake Keystone v3 test service (testservices/identityservice/v3appcred.go) and a test suite (identity/v3appcred_test.go) covering a full auth round-trip, invalid secret, missing id/secret, and that no scope is ever put on the wire even if the caller sets TenantName/Domain by mistake.
newClient() only special-cased AuthUserPassV3 for Keystone's v3 '/auth/tokens' suffix; every other mode, including the new AuthApplicationCredentialV3, fell through to the default '/tokens' (Keystone v2) suffix. The v3 application-credential request was reaching the wrong endpoint entirely (and would 404 against a real v3-only Keystone), never exercised by the identity-package tests since those call V3AppCred.Auth() directly with a pre-built URL, bypassing this client-level suffixing. Found via a consumer's (spread) end-to-end smoke test that drives the full client.NewClient(...).Authenticate() path instead of just the Authenticator in isolation.
time.Time's UnmarshalJSON requires strict RFC3339 (trailing Z or numeric offset). Keystone's response to application-credential auth on at least one deployment omits that suffix on expires_at/issued_at (e.g. "2026-10-08T19:27:05.092894", no Z), which made authentication fail outright even though these two fields are never read afterwards in v3KeystoneAuth's returned AuthDetails. Add flexibleTime: try RFC3339Nano first, fall back to the same layout without a timezone offset (assumed UTC) if that fails.
Member
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment (1 issue, 1 suggestion)
Critical
- None.
Warnings
- identity/identity.go:221 — Make
CompleteCredentialsFromEnvrecognise application-credential authentication. It still treatsUserandSecretsas required because only the two new application-credential fields are tagged optional, so a standard environment containing onlyOS_AUTH_URL,OS_APPLICATION_CREDENTIAL_ID, andOS_APPLICATION_CREDENTIAL_SECRETfails validation beforeV3AppCred.Authcan run. Add mode-aware completion/validation that requires the appropriate alternative credential set without weakening the existing username/password checks, and cover that environment-only path.
Suggestions
- identity/v3userpass.go:171 — Add a regression test that authenticates from a token response whose
issued_atandexpires_atomit a timezone. The fake service serialisestime.Now()in RFC3339 form, so the present app-credential tests never execute the newParseInLocationfallback.
Architecture and Alternative Approaches
- Keep the shared v3 request and response handling as the common boundary; an authentication-mode-aware credentials validator is sufficient and avoids duplicating environment parsing in
V3AppCred.
Looks Good
V3AppCred.Authrejects incomplete application credentials before making a request.- The request deliberately omits project scope, matching Keystone application-credential semantics.
- The client selects the v3 token endpoint for the new mode.
Reviewed by lee-van-cleef[bot]
CompleteCredentialsFromEnv treated User/Secrets as always-required, so a standard environment containing only OS_AUTH_URL, OS_APPLICATION_CREDENTIAL_ID, and OS_APPLICATION_CREDENTIAL_SECRET failed validation before V3AppCred.Auth ever ran. Application credentials carry no username/password; skip the User/Secrets check when either application-credential field is present, leaving the existing username/password/keypair validation unchanged. Also add a regression test for the flexibleTime no-timezone fallback: the fake V3AppCred service always serialises expires_at/ issued_at via time.Time (always RFC3339, always a trailing Z), so the existing app-credential tests never exercised the ParseInLocation fallback added in 3e75941. Serve a handwritten raw response instead to cover that path end to end. Addresses review feedback from @SimonRichardson on PR go-goose#109.
SimonRichardson
approved these changes
Oct 15, 2026
wallyworld
approved these changes
Oct 16, 2026
Member
There was a problem hiding this comment.
Thank you - let's fix the warning before landing though.
Contributor
Author
|
Thanks for the review, @wallyworld the warning should be fixed |
simonepelosi
added a commit
to simonepelosi/spread
that referenced
this pull request
Oct 16, 2026
goose v5.2.0 ships the application-credential auth that this branch used to get from a fork (go-goose/goose#109). Require the release and drop the replace directive that pinned github.com/simonepelosi/goose. The goose API spread uses is unchanged: the other goose packages are identical to the pinned fork commit. v5.2.0 also accepts Keystone token timestamps without a timezone offset, which some deployments return for application-credential tokens.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goose currently supports username+password auth (v2 and v3) and EC2-style access/secret key pairs, but no way to authenticate with an OpenStack application credential
(https://docs.openstack.org/api-ref/identity/v3/#application-credentials) (id+secret) — a distinct Keystone v3 auth method with its own request shape and no explicit project scope (the credential is already scoped to a project at creation time; Keystone rejects a request that both uses an application credential and asks for an explicit scope).