-
Notifications
You must be signed in to change notification settings - Fork 134
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
Refactor the way we access Config
#222
Conversation
Blah, I hoisted the verifier creation above the place we augment the system's TLS certs 🤔 I'll sort this out in a bit. |
This change starts to refactor the way we interact with theh Fulcio `Config` to support a forthcoming change to support what I've taken to calling "meta URLs". I want to allow the public Fulcio instance to support the following *classes* of Issuer URL, which are exposed for EKS and GKE clusters: * https://oidc.eks.us-west-2.amazonaws.com/id/B02C93B6A2D30341AD01E1B6D48164CB * https://container.googleapis.com/v1/projects/mattmoor-credit/locations/us-west1-b/clusters/tenant-cluster To this end, I want to support adding to the Fulcio configuration "meta URLs" of the form (still a strawman): ``` https://oidc.eks.*.amazonaws.com/id/* https://container.googleapis.com/v1/projects/*/locations/*/clusters/* ``` However, in this change, I'm still just consolidating how we access the OIDC issuer information in `FulcioConfig` so that lookups are performed via a new `GetIssuer` method, and verifiers are instantiated via `GetVerifier()`. The only remaining place I see iterating over `cfg.OIDCIssuers` is for the `realm` in the `WWW-Authenticate` header of `401` responses (seems right to not include the meta URLs here). There are also still a handful of places handling OIDCIssuers directly, but most are in `config.go`, or associated validation logic (e.g. `federation/main.go`). Related: sigstore#212 Signed-off-by: Matt Moore <[email protected]>
91b7c50
to
bbd4c56
Compare
pkg/config/config.go
Outdated
return nil, err | ||
} | ||
|
||
if _, ok := cfg.OIDCIssuers["https://kubernetes.default.svc"]; ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe one nit: it's a bit surprising for a ParseConfig
function to do things other than Parse the input config. Could we split this out into a separate function after the Parsing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was putting it here because it was public, but it isn't called elsewhere, so I made it private and moved the logic back into it's one callsite.
Signed-off-by: Matt Moore <[email protected]>
This change starts to refactor the way we interact with theh Fulcio
Config
to support a forthcoming change to support what I've taken to calling "meta URLs".
I want to allow the public Fulcio instance to support the following classes
of Issuer URL, which are exposed for EKS and GKE clusters:
To this end, I want to support adding to the Fulcio configuration "meta URLs"
of the form (still a strawman):
However, in this change, I'm still just consolidating how we access the OIDC
issuer information in
FulcioConfig
so that lookups are performed via a newGetIssuer
method, and verifiers are instantiated viaGetVerifier()
.The only remaining place I see iterating over
cfg.OIDCIssuers
is for therealm
in theWWW-Authenticate
header of401
responses (seems right tonot include the meta URLs here). There are also still a handful of places
handling OIDCIssuers directly, but most are in
config.go
, or associatedvalidation logic (e.g.
federation/main.go
).Signed-off-by: Matt Moore [email protected]
Ticket Link
Related: #212
Release Note