From 82ae98d9d0653ca0b73ec94895ce748c1988674b Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Tue, 15 Dec 2020 09:13:01 -0500 Subject: [PATCH] Set secret names on OIDCProvider status field We believe this API is more forwards compatible with future secrets management use cases. The implementation is a cry for help, but I was trying to follow the previously established pattern of encapsulating the secret generation functionality to a single group of packages. This commit makes a breaking change to the current OIDCProvider API, but that OIDCProvider API was added after the latest release, so it is technically still in development until we release, and therefore we can continue to thrash on it. I also took this opportunity to make some things private that didn't need to be public. Signed-off-by: Andrew Keesler --- .../v1alpha1/types_oidcprovider.go.tmpl | 30 +++- cmd/pinniped-supervisor/main.go | 6 + ...supervisor.pinniped.dev_oidcproviders.yaml | 56 ++++++-- generated/1.17/README.adoc | 22 ++- .../config/v1alpha1/types_oidcprovider.go | 30 +++- .../config/v1alpha1/zz_generated.deepcopy.go | 22 ++- ...supervisor.pinniped.dev_oidcproviders.yaml | 56 ++++++-- generated/1.18/README.adoc | 22 ++- .../config/v1alpha1/types_oidcprovider.go | 30 +++- .../config/v1alpha1/zz_generated.deepcopy.go | 22 ++- ...supervisor.pinniped.dev_oidcproviders.yaml | 56 ++++++-- generated/1.19/README.adoc | 22 ++- .../config/v1alpha1/types_oidcprovider.go | 30 +++- .../config/v1alpha1/zz_generated.deepcopy.go | 22 ++- ...supervisor.pinniped.dev_oidcproviders.yaml | 56 ++++++-- .../supervisorconfig/generator/generator.go | 8 +- .../generator/oidc_provider_secrets.go | 40 +++++- .../generator/oidc_provider_secrets_test.go | 126 +++++++++++++---- .../generator/secret_helper.go | 54 ++++++-- .../generator/secret_helper_test.go | 129 ++++++++++++------ .../generator/supervisor_secrets.go | 4 +- .../generator/supervisor_secrets_test.go | 4 +- .../supervisorconfig/jwks_observer.go | 2 +- .../supervisorconfig/jwks_observer_test.go | 22 ++- .../supervisorconfig/jwks_writer.go | 10 +- .../supervisorconfig/jwks_writer_test.go | 4 +- .../mocksecrethelper/mocksecrethelper.go | 14 +- test/integration/supervisor_secrets_test.go | 8 +- 28 files changed, 724 insertions(+), 183 deletions(-) diff --git a/apis/supervisor/config/v1alpha1/types_oidcprovider.go.tmpl b/apis/supervisor/config/v1alpha1/types_oidcprovider.go.tmpl index 908470f01..2e31219e0 100644 --- a/apis/supervisor/config/v1alpha1/types_oidcprovider.go.tmpl +++ b/apis/supervisor/config/v1alpha1/types_oidcprovider.go.tmpl @@ -59,6 +59,30 @@ type OIDCProviderSpec struct { TLS *OIDCProviderTLSSpec `json:"tls,omitempty"` } +// OIDCProviderSecrets holds information about this OIDC Provider's secrets. +type OIDCProviderSecrets struct { + // JWKS holds the name of the corev1.Secret in which this OIDC Provider's signing/verification keys are + // stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. + // +optional + JWKS corev1.LocalObjectReference `json:"jwks,omitempty"` + + // TokenSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // signing tokens is stored. + // +optional + TokenSigningKey corev1.LocalObjectReference `json:"tokenSigningKey,omitempty"` + + // StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // signing state parameters is stored. + // +optional + StateSigningKey corev1.LocalObjectReference `json:"stateSigningKey,omitempty"` + + // StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // encrypting state parameters is stored. + // +optional + StateEncryptionKey corev1.LocalObjectReference `json:"stateEncryptionKey,omitempty"` +} + // OIDCProviderStatus is a struct that describes the actual state of an OIDC Provider. type OIDCProviderStatus struct { // Status holds an enum that describes the state of this OIDC Provider. Note that this Status can @@ -76,11 +100,9 @@ type OIDCProviderStatus struct { // +optional LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` - // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys - // are stored. If it is empty, then the signing/verification keys are either unknown or they don't - // exist. + // Secrets contains information about this OIDC Provider's secrets. // +optional - JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` + Secrets OIDCProviderSecrets `json:"secrets,omitempty"` } // OIDCProvider describes the configuration of an OIDC provider. diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index 07cec6f27..461eea56e 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -167,12 +167,14 @@ func startControllers( "pinniped-oidc-provider-hmac-key-", cfg.Labels, rand.Reader, + generator.SecretUsageTokenSigningKey, func(oidcProviderIssuer string, symmetricKey []byte) { plog.Debug("setting hmac secret", "issuer", oidcProviderIssuer) secretCache.SetTokenHMACKey(oidcProviderIssuer, symmetricKey) }, ), kubeClient, + pinnipedClient, secretInformer, opInformer, controllerlib.WithInformer, @@ -185,12 +187,14 @@ func startControllers( "pinniped-oidc-provider-upstream-state-signature-key-", cfg.Labels, rand.Reader, + generator.SecretUsageStateSigningKey, func(oidcProviderIssuer string, symmetricKey []byte) { plog.Debug("setting state signature key", "issuer", oidcProviderIssuer) secretCache.SetStateEncoderHashKey(oidcProviderIssuer, symmetricKey) }, ), kubeClient, + pinnipedClient, secretInformer, opInformer, controllerlib.WithInformer, @@ -203,12 +207,14 @@ func startControllers( "pinniped-oidc-provider-upstream-state-encryption-key-", cfg.Labels, rand.Reader, + generator.SecretUsageStateEncryptionKey, func(oidcProviderIssuer string, symmetricKey []byte) { plog.Debug("setting state encryption key", "issuer", oidcProviderIssuer) secretCache.SetStateEncoderBlockKey(oidcProviderIssuer, symmetricKey) }, ), kubeClient, + pinnipedClient, secretInformer, opInformer, controllerlib.WithInformer, diff --git a/deploy/supervisor/config.supervisor.pinniped.dev_oidcproviders.yaml b/deploy/supervisor/config.supervisor.pinniped.dev_oidcproviders.yaml index 6ff3a42b1..18771821b 100644 --- a/deploy/supervisor/config.supervisor.pinniped.dev_oidcproviders.yaml +++ b/deploy/supervisor/config.supervisor.pinniped.dev_oidcproviders.yaml @@ -81,17 +81,6 @@ spec: status: description: Status of the OIDC provider. properties: - jwksSecret: - description: JWKSSecret holds the name of the secret in which this - OIDC Provider's signing/verification keys are stored. If it is empty, - then the signing/verification keys are either unknown or they don't - exist. - properties: - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - type: object lastUpdateTime: description: LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior @@ -101,6 +90,51 @@ spec: message: description: Message provides human-readable details about the Status. type: string + secrets: + description: Secrets contains information about this OIDC Provider's + secrets. + properties: + jwks: + description: JWKS holds the name of the corev1.Secret in which + this OIDC Provider's signing/verification keys are stored. If + it is empty, then the signing/verification keys are either unknown + or they don't exist. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + stateEncryptionKey: + description: StateSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for encrypting state parameters + is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + stateSigningKey: + description: StateSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for signing state parameters + is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + tokenSigningKey: + description: TokenSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for signing tokens is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + type: object status: description: Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. diff --git a/generated/1.17/README.adoc b/generated/1.17/README.adoc index 0ccdd1df9..5af68c1e3 100644 --- a/generated/1.17/README.adoc +++ b/generated/1.17/README.adoc @@ -304,6 +304,26 @@ OIDCProvider describes the configuration of an OIDC provider. +[id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-config-v1alpha1-oidcprovidersecrets"] +==== OIDCProviderSecrets + +OIDCProviderSecrets holds information about this OIDC Provider's secrets. + +.Appears In: +**** +- xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-config-v1alpha1-oidcproviderstatus[$$OIDCProviderStatus$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`jwks`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | JWKS holds the name of the corev1.Secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist. +| *`tokenSigningKey`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | TokenSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for signing tokens is stored. +| *`stateSigningKey`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for signing state parameters is stored. +| *`stateEncryptionKey`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for encrypting state parameters is stored. +|=== + + [id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-config-v1alpha1-oidcproviderspec"] ==== OIDCProviderSpec @@ -339,7 +359,7 @@ OIDCProviderStatus is a struct that describes the actual state of an OIDC Provid | *`status`* __OIDCProviderStatusCondition__ | Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. | *`message`* __string__ | Message provides human-readable details about the Status. | *`lastUpdateTime`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#time-v1-meta[$$Time$$]__ | LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior with respect to the empty metav1.Time value (see https://github.com/kubernetes/kubernetes/issues/86811). -| *`jwksSecret`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist. +| *`secrets`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-config-v1alpha1-oidcprovidersecrets[$$OIDCProviderSecrets$$]__ | Secrets contains information about this OIDC Provider's secrets. |=== diff --git a/generated/1.17/apis/supervisor/config/v1alpha1/types_oidcprovider.go b/generated/1.17/apis/supervisor/config/v1alpha1/types_oidcprovider.go index 908470f01..2e31219e0 100644 --- a/generated/1.17/apis/supervisor/config/v1alpha1/types_oidcprovider.go +++ b/generated/1.17/apis/supervisor/config/v1alpha1/types_oidcprovider.go @@ -59,6 +59,30 @@ type OIDCProviderSpec struct { TLS *OIDCProviderTLSSpec `json:"tls,omitempty"` } +// OIDCProviderSecrets holds information about this OIDC Provider's secrets. +type OIDCProviderSecrets struct { + // JWKS holds the name of the corev1.Secret in which this OIDC Provider's signing/verification keys are + // stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. + // +optional + JWKS corev1.LocalObjectReference `json:"jwks,omitempty"` + + // TokenSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // signing tokens is stored. + // +optional + TokenSigningKey corev1.LocalObjectReference `json:"tokenSigningKey,omitempty"` + + // StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // signing state parameters is stored. + // +optional + StateSigningKey corev1.LocalObjectReference `json:"stateSigningKey,omitempty"` + + // StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // encrypting state parameters is stored. + // +optional + StateEncryptionKey corev1.LocalObjectReference `json:"stateEncryptionKey,omitempty"` +} + // OIDCProviderStatus is a struct that describes the actual state of an OIDC Provider. type OIDCProviderStatus struct { // Status holds an enum that describes the state of this OIDC Provider. Note that this Status can @@ -76,11 +100,9 @@ type OIDCProviderStatus struct { // +optional LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` - // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys - // are stored. If it is empty, then the signing/verification keys are either unknown or they don't - // exist. + // Secrets contains information about this OIDC Provider's secrets. // +optional - JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` + Secrets OIDCProviderSecrets `json:"secrets,omitempty"` } // OIDCProvider describes the configuration of an OIDC provider. diff --git a/generated/1.17/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go b/generated/1.17/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go index f208d4d05..0cfc17a42 100644 --- a/generated/1.17/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.17/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go @@ -72,6 +72,26 @@ func (in *OIDCProviderList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OIDCProviderSecrets) DeepCopyInto(out *OIDCProviderSecrets) { + *out = *in + out.JWKS = in.JWKS + out.TokenSigningKey = in.TokenSigningKey + out.StateSigningKey = in.StateSigningKey + out.StateEncryptionKey = in.StateEncryptionKey + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDCProviderSecrets. +func (in *OIDCProviderSecrets) DeepCopy() *OIDCProviderSecrets { + if in == nil { + return nil + } + out := new(OIDCProviderSecrets) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OIDCProviderSpec) DeepCopyInto(out *OIDCProviderSpec) { *out = *in @@ -100,7 +120,7 @@ func (in *OIDCProviderStatus) DeepCopyInto(out *OIDCProviderStatus) { in, out := &in.LastUpdateTime, &out.LastUpdateTime *out = (*in).DeepCopy() } - out.JWKSSecret = in.JWKSSecret + out.Secrets = in.Secrets return } diff --git a/generated/1.17/crds/config.supervisor.pinniped.dev_oidcproviders.yaml b/generated/1.17/crds/config.supervisor.pinniped.dev_oidcproviders.yaml index 6ff3a42b1..18771821b 100644 --- a/generated/1.17/crds/config.supervisor.pinniped.dev_oidcproviders.yaml +++ b/generated/1.17/crds/config.supervisor.pinniped.dev_oidcproviders.yaml @@ -81,17 +81,6 @@ spec: status: description: Status of the OIDC provider. properties: - jwksSecret: - description: JWKSSecret holds the name of the secret in which this - OIDC Provider's signing/verification keys are stored. If it is empty, - then the signing/verification keys are either unknown or they don't - exist. - properties: - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - type: object lastUpdateTime: description: LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior @@ -101,6 +90,51 @@ spec: message: description: Message provides human-readable details about the Status. type: string + secrets: + description: Secrets contains information about this OIDC Provider's + secrets. + properties: + jwks: + description: JWKS holds the name of the corev1.Secret in which + this OIDC Provider's signing/verification keys are stored. If + it is empty, then the signing/verification keys are either unknown + or they don't exist. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + stateEncryptionKey: + description: StateSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for encrypting state parameters + is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + stateSigningKey: + description: StateSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for signing state parameters + is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + tokenSigningKey: + description: TokenSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for signing tokens is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + type: object status: description: Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. diff --git a/generated/1.18/README.adoc b/generated/1.18/README.adoc index 97042b25d..f0bca6a4e 100644 --- a/generated/1.18/README.adoc +++ b/generated/1.18/README.adoc @@ -304,6 +304,26 @@ OIDCProvider describes the configuration of an OIDC provider. +[id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-config-v1alpha1-oidcprovidersecrets"] +==== OIDCProviderSecrets + +OIDCProviderSecrets holds information about this OIDC Provider's secrets. + +.Appears In: +**** +- xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-config-v1alpha1-oidcproviderstatus[$$OIDCProviderStatus$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`jwks`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | JWKS holds the name of the corev1.Secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist. +| *`tokenSigningKey`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | TokenSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for signing tokens is stored. +| *`stateSigningKey`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for signing state parameters is stored. +| *`stateEncryptionKey`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for encrypting state parameters is stored. +|=== + + [id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-config-v1alpha1-oidcproviderspec"] ==== OIDCProviderSpec @@ -339,7 +359,7 @@ OIDCProviderStatus is a struct that describes the actual state of an OIDC Provid | *`status`* __OIDCProviderStatusCondition__ | Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. | *`message`* __string__ | Message provides human-readable details about the Status. | *`lastUpdateTime`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#time-v1-meta[$$Time$$]__ | LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior with respect to the empty metav1.Time value (see https://github.com/kubernetes/kubernetes/issues/86811). -| *`jwksSecret`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist. +| *`secrets`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-config-v1alpha1-oidcprovidersecrets[$$OIDCProviderSecrets$$]__ | Secrets contains information about this OIDC Provider's secrets. |=== diff --git a/generated/1.18/apis/supervisor/config/v1alpha1/types_oidcprovider.go b/generated/1.18/apis/supervisor/config/v1alpha1/types_oidcprovider.go index 908470f01..2e31219e0 100644 --- a/generated/1.18/apis/supervisor/config/v1alpha1/types_oidcprovider.go +++ b/generated/1.18/apis/supervisor/config/v1alpha1/types_oidcprovider.go @@ -59,6 +59,30 @@ type OIDCProviderSpec struct { TLS *OIDCProviderTLSSpec `json:"tls,omitempty"` } +// OIDCProviderSecrets holds information about this OIDC Provider's secrets. +type OIDCProviderSecrets struct { + // JWKS holds the name of the corev1.Secret in which this OIDC Provider's signing/verification keys are + // stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. + // +optional + JWKS corev1.LocalObjectReference `json:"jwks,omitempty"` + + // TokenSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // signing tokens is stored. + // +optional + TokenSigningKey corev1.LocalObjectReference `json:"tokenSigningKey,omitempty"` + + // StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // signing state parameters is stored. + // +optional + StateSigningKey corev1.LocalObjectReference `json:"stateSigningKey,omitempty"` + + // StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // encrypting state parameters is stored. + // +optional + StateEncryptionKey corev1.LocalObjectReference `json:"stateEncryptionKey,omitempty"` +} + // OIDCProviderStatus is a struct that describes the actual state of an OIDC Provider. type OIDCProviderStatus struct { // Status holds an enum that describes the state of this OIDC Provider. Note that this Status can @@ -76,11 +100,9 @@ type OIDCProviderStatus struct { // +optional LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` - // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys - // are stored. If it is empty, then the signing/verification keys are either unknown or they don't - // exist. + // Secrets contains information about this OIDC Provider's secrets. // +optional - JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` + Secrets OIDCProviderSecrets `json:"secrets,omitempty"` } // OIDCProvider describes the configuration of an OIDC provider. diff --git a/generated/1.18/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go b/generated/1.18/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go index f208d4d05..0cfc17a42 100644 --- a/generated/1.18/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.18/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go @@ -72,6 +72,26 @@ func (in *OIDCProviderList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OIDCProviderSecrets) DeepCopyInto(out *OIDCProviderSecrets) { + *out = *in + out.JWKS = in.JWKS + out.TokenSigningKey = in.TokenSigningKey + out.StateSigningKey = in.StateSigningKey + out.StateEncryptionKey = in.StateEncryptionKey + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDCProviderSecrets. +func (in *OIDCProviderSecrets) DeepCopy() *OIDCProviderSecrets { + if in == nil { + return nil + } + out := new(OIDCProviderSecrets) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OIDCProviderSpec) DeepCopyInto(out *OIDCProviderSpec) { *out = *in @@ -100,7 +120,7 @@ func (in *OIDCProviderStatus) DeepCopyInto(out *OIDCProviderStatus) { in, out := &in.LastUpdateTime, &out.LastUpdateTime *out = (*in).DeepCopy() } - out.JWKSSecret = in.JWKSSecret + out.Secrets = in.Secrets return } diff --git a/generated/1.18/crds/config.supervisor.pinniped.dev_oidcproviders.yaml b/generated/1.18/crds/config.supervisor.pinniped.dev_oidcproviders.yaml index 6ff3a42b1..18771821b 100644 --- a/generated/1.18/crds/config.supervisor.pinniped.dev_oidcproviders.yaml +++ b/generated/1.18/crds/config.supervisor.pinniped.dev_oidcproviders.yaml @@ -81,17 +81,6 @@ spec: status: description: Status of the OIDC provider. properties: - jwksSecret: - description: JWKSSecret holds the name of the secret in which this - OIDC Provider's signing/verification keys are stored. If it is empty, - then the signing/verification keys are either unknown or they don't - exist. - properties: - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - type: object lastUpdateTime: description: LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior @@ -101,6 +90,51 @@ spec: message: description: Message provides human-readable details about the Status. type: string + secrets: + description: Secrets contains information about this OIDC Provider's + secrets. + properties: + jwks: + description: JWKS holds the name of the corev1.Secret in which + this OIDC Provider's signing/verification keys are stored. If + it is empty, then the signing/verification keys are either unknown + or they don't exist. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + stateEncryptionKey: + description: StateSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for encrypting state parameters + is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + stateSigningKey: + description: StateSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for signing state parameters + is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + tokenSigningKey: + description: TokenSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for signing tokens is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + type: object status: description: Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. diff --git a/generated/1.19/README.adoc b/generated/1.19/README.adoc index edda33b88..ec9176fe3 100644 --- a/generated/1.19/README.adoc +++ b/generated/1.19/README.adoc @@ -304,6 +304,26 @@ OIDCProvider describes the configuration of an OIDC provider. +[id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-config-v1alpha1-oidcprovidersecrets"] +==== OIDCProviderSecrets + +OIDCProviderSecrets holds information about this OIDC Provider's secrets. + +.Appears In: +**** +- xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-config-v1alpha1-oidcproviderstatus[$$OIDCProviderStatus$$] +**** + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`jwks`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | JWKS holds the name of the corev1.Secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist. +| *`tokenSigningKey`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | TokenSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for signing tokens is stored. +| *`stateSigningKey`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for signing state parameters is stored. +| *`stateEncryptionKey`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for encrypting state parameters is stored. +|=== + + [id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-config-v1alpha1-oidcproviderspec"] ==== OIDCProviderSpec @@ -339,7 +359,7 @@ OIDCProviderStatus is a struct that describes the actual state of an OIDC Provid | *`status`* __OIDCProviderStatusCondition__ | Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. | *`message`* __string__ | Message provides human-readable details about the Status. | *`lastUpdateTime`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#time-v1-meta[$$Time$$]__ | LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior with respect to the empty metav1.Time value (see https://github.com/kubernetes/kubernetes/issues/86811). -| *`jwksSecret`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#localobjectreference-v1-core[$$LocalObjectReference$$]__ | JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys are stored. If it is empty, then the signing/verification keys are either unknown or they don't exist. +| *`secrets`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-config-v1alpha1-oidcprovidersecrets[$$OIDCProviderSecrets$$]__ | Secrets contains information about this OIDC Provider's secrets. |=== diff --git a/generated/1.19/apis/supervisor/config/v1alpha1/types_oidcprovider.go b/generated/1.19/apis/supervisor/config/v1alpha1/types_oidcprovider.go index 908470f01..2e31219e0 100644 --- a/generated/1.19/apis/supervisor/config/v1alpha1/types_oidcprovider.go +++ b/generated/1.19/apis/supervisor/config/v1alpha1/types_oidcprovider.go @@ -59,6 +59,30 @@ type OIDCProviderSpec struct { TLS *OIDCProviderTLSSpec `json:"tls,omitempty"` } +// OIDCProviderSecrets holds information about this OIDC Provider's secrets. +type OIDCProviderSecrets struct { + // JWKS holds the name of the corev1.Secret in which this OIDC Provider's signing/verification keys are + // stored. If it is empty, then the signing/verification keys are either unknown or they don't + // exist. + // +optional + JWKS corev1.LocalObjectReference `json:"jwks,omitempty"` + + // TokenSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // signing tokens is stored. + // +optional + TokenSigningKey corev1.LocalObjectReference `json:"tokenSigningKey,omitempty"` + + // StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // signing state parameters is stored. + // +optional + StateSigningKey corev1.LocalObjectReference `json:"stateSigningKey,omitempty"` + + // StateSigningKey holds the name of the corev1.Secret in which this OIDC Provider's key for + // encrypting state parameters is stored. + // +optional + StateEncryptionKey corev1.LocalObjectReference `json:"stateEncryptionKey,omitempty"` +} + // OIDCProviderStatus is a struct that describes the actual state of an OIDC Provider. type OIDCProviderStatus struct { // Status holds an enum that describes the state of this OIDC Provider. Note that this Status can @@ -76,11 +100,9 @@ type OIDCProviderStatus struct { // +optional LastUpdateTime *metav1.Time `json:"lastUpdateTime,omitempty"` - // JWKSSecret holds the name of the secret in which this OIDC Provider's signing/verification keys - // are stored. If it is empty, then the signing/verification keys are either unknown or they don't - // exist. + // Secrets contains information about this OIDC Provider's secrets. // +optional - JWKSSecret corev1.LocalObjectReference `json:"jwksSecret,omitempty"` + Secrets OIDCProviderSecrets `json:"secrets,omitempty"` } // OIDCProvider describes the configuration of an OIDC provider. diff --git a/generated/1.19/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go b/generated/1.19/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go index f208d4d05..0cfc17a42 100644 --- a/generated/1.19/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.19/apis/supervisor/config/v1alpha1/zz_generated.deepcopy.go @@ -72,6 +72,26 @@ func (in *OIDCProviderList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OIDCProviderSecrets) DeepCopyInto(out *OIDCProviderSecrets) { + *out = *in + out.JWKS = in.JWKS + out.TokenSigningKey = in.TokenSigningKey + out.StateSigningKey = in.StateSigningKey + out.StateEncryptionKey = in.StateEncryptionKey + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDCProviderSecrets. +func (in *OIDCProviderSecrets) DeepCopy() *OIDCProviderSecrets { + if in == nil { + return nil + } + out := new(OIDCProviderSecrets) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OIDCProviderSpec) DeepCopyInto(out *OIDCProviderSpec) { *out = *in @@ -100,7 +120,7 @@ func (in *OIDCProviderStatus) DeepCopyInto(out *OIDCProviderStatus) { in, out := &in.LastUpdateTime, &out.LastUpdateTime *out = (*in).DeepCopy() } - out.JWKSSecret = in.JWKSSecret + out.Secrets = in.Secrets return } diff --git a/generated/1.19/crds/config.supervisor.pinniped.dev_oidcproviders.yaml b/generated/1.19/crds/config.supervisor.pinniped.dev_oidcproviders.yaml index 6ff3a42b1..18771821b 100644 --- a/generated/1.19/crds/config.supervisor.pinniped.dev_oidcproviders.yaml +++ b/generated/1.19/crds/config.supervisor.pinniped.dev_oidcproviders.yaml @@ -81,17 +81,6 @@ spec: status: description: Status of the OIDC provider. properties: - jwksSecret: - description: JWKSSecret holds the name of the secret in which this - OIDC Provider's signing/verification keys are stored. If it is empty, - then the signing/verification keys are either unknown or they don't - exist. - properties: - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid?' - type: string - type: object lastUpdateTime: description: LastUpdateTime holds the time at which the Status was last updated. It is a pointer to get around some undesirable behavior @@ -101,6 +90,51 @@ spec: message: description: Message provides human-readable details about the Status. type: string + secrets: + description: Secrets contains information about this OIDC Provider's + secrets. + properties: + jwks: + description: JWKS holds the name of the corev1.Secret in which + this OIDC Provider's signing/verification keys are stored. If + it is empty, then the signing/verification keys are either unknown + or they don't exist. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + stateEncryptionKey: + description: StateSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for encrypting state parameters + is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + stateSigningKey: + description: StateSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for signing state parameters + is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + tokenSigningKey: + description: TokenSigningKey holds the name of the corev1.Secret + in which this OIDC Provider's key for signing tokens is stored. + properties: + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + type: object + type: object status: description: Status holds an enum that describes the state of this OIDC Provider. Note that this Status can represent success or failure. diff --git a/internal/controller/supervisorconfig/generator/generator.go b/internal/controller/supervisorconfig/generator/generator.go index 3aeed9554..26328caac 100644 --- a/internal/controller/supervisorconfig/generator/generator.go +++ b/internal/controller/supervisorconfig/generator/generator.go @@ -27,11 +27,11 @@ func generateSymmetricKey() ([]byte, error) { } func isValid(secret *corev1.Secret) bool { - if secret.Type != SymmetricSecretType { + if secret.Type != symmetricSecretType { return false } - data, ok := secret.Data[SymmetricSecretDataKey] + data, ok := secret.Data[symmetricSecretDataKey] if !ok { return false } @@ -49,7 +49,7 @@ func secretDataFunc() (map[string][]byte, error) { } return map[string][]byte{ - SymmetricSecretDataKey: symmetricKey, + symmetricSecretDataKey: symmetricKey, }, nil } @@ -73,7 +73,7 @@ func generateSecret(namespace, name string, labels map[string]string, secretData }, Labels: labels, }, - Type: SymmetricSecretType, + Type: symmetricSecretType, Data: secretData, }, nil } diff --git a/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go b/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go index 01f0c4912..aeada106d 100644 --- a/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go +++ b/internal/controller/supervisorconfig/generator/oidc_provider_secrets.go @@ -6,6 +6,7 @@ package generator import ( "context" "fmt" + "reflect" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -16,6 +17,7 @@ import ( "k8s.io/klog/v2" configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1" + pinnipedclientset "go.pinniped.dev/generated/1.19/client/supervisor/clientset/versioned" configinformers "go.pinniped.dev/generated/1.19/client/supervisor/informers/externalversions/config/v1alpha1" pinnipedcontroller "go.pinniped.dev/internal/controller" "go.pinniped.dev/internal/controllerlib" @@ -25,6 +27,7 @@ import ( type oidcProviderSecretsController struct { secretHelper SecretHelper kubeClient kubernetes.Interface + pinnipedClient pinnipedclientset.Interface opcInformer configinformers.OIDCProviderInformer secretInformer corev1informers.SecretInformer } @@ -35,6 +38,7 @@ type oidcProviderSecretsController struct { func NewOIDCProviderSecretsController( secretHelper SecretHelper, kubeClient kubernetes.Interface, + pinnipedClient pinnipedclientset.Interface, secretInformer corev1informers.SecretInformer, opcInformer configinformers.OIDCProviderInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, @@ -45,6 +49,7 @@ func NewOIDCProviderSecretsController( Syncer: &oidcProviderSecretsController{ secretHelper: secretHelper, kubeClient: kubeClient, + pinnipedClient: pinnipedClient, secretInformer: secretInformer, opcInformer: opcInformer, }, @@ -116,7 +121,13 @@ func (c *oidcProviderSecretsController) Sync(ctx controllerlib.Context) error { "secret", klog.KObj(existingSecret), ) - c.secretHelper.Notify(op, existingSecret) + + op = c.secretHelper.ObserveActiveSecretAndUpdateParentOIDCProvider(op, existingSecret) + if err := c.updateOIDCProvider(ctx.Context, op); err != nil { + return fmt.Errorf("failed to update oidcprovider: %w", err) + } + plog.Debug("updated oidcprovider", "oidcprovider", klog.KObj(op), "secret", klog.KObj(newSecret)) + return nil } @@ -127,7 +138,11 @@ func (c *oidcProviderSecretsController) Sync(ctx controllerlib.Context) error { } plog.Debug("created/updated secret", "oidcprovider", klog.KObj(op), "secret", klog.KObj(newSecret)) - c.secretHelper.Notify(op, newSecret) + op = c.secretHelper.ObserveActiveSecretAndUpdateParentOIDCProvider(op, newSecret) + if err := c.updateOIDCProvider(ctx.Context, op); err != nil { + return fmt.Errorf("failed to update oidcprovider: %w", err) + } + plog.Debug("updated oidcprovider", "oidcprovider", klog.KObj(op), "secret", klog.KObj(newSecret)) return nil } @@ -195,3 +210,24 @@ func (c *oidcProviderSecretsController) createOrUpdateSecret( return err }) } + +func (c *oidcProviderSecretsController) updateOIDCProvider( + ctx context.Context, + newOP *configv1alpha1.OIDCProvider, +) error { + opcClient := c.pinnipedClient.ConfigV1alpha1().OIDCProviders(newOP.Namespace) + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + oldOP, err := opcClient.Get(ctx, newOP.Name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get oidcprovider %s/%s: %w", newOP.Namespace, newOP.Name, err) + } + + if reflect.DeepEqual(newOP.Status.Secrets, oldOP.Status.Secrets) { + return nil + } + + oldOP.Status.Secrets = newOP.Status.Secrets + _, err = opcClient.Update(ctx, oldOP, metav1.UpdateOptions{}) + return err + }) +} diff --git a/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go b/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go index 68cbc4bf2..e888c7c23 100644 --- a/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go +++ b/internal/controller/supervisorconfig/generator/oidc_provider_secrets_test.go @@ -158,6 +158,7 @@ func TestOIDCProviderControllerFilterSecret(t *testing.T) { _ = NewOIDCProviderSecretsController( secretHelper, nil, // kubeClient, not needed + nil, // pinnipedClient, not needed secretInformer, opcInformer, withInformer.WithInformer, @@ -216,6 +217,7 @@ func TestNewOIDCProviderSecretsControllerFilterOPC(t *testing.T) { _ = NewOIDCProviderSecretsController( secretHelper, nil, // kubeClient, not needed + nil, // pinnipedClient, not needed secretInformer, opcInformer, withInformer.WithInformer, @@ -291,6 +293,9 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { }, } + goodOPWithStatus := goodOP.DeepCopy() + goodOPWithStatus.Status.Secrets.TokenSigningKey.Name = goodSecret.Name + invalidSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: secretName, @@ -309,26 +314,24 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { }, } - once := sync.Once{} - tests := []struct { name string storage func(**configv1alpha1.OIDCProvider, **corev1.Secret) - client func(*kubernetesfake.Clientset) + client func(*pinnipedfake.Clientset, *kubernetesfake.Clientset) secretHelper func(*mocksecrethelper.MockSecretHelper) - wantSecretActions []kubetesting.Action wantOPActions []kubetesting.Action + wantSecretActions []kubetesting.Action wantError string }{ { - name: "OIDCProvider exists and secret does not exist", + name: "OIDCProvider does not exist and secret does not exist", storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) { *op = nil *s = nil }, }, { - name: "OIDCProvider exists and secret exists", + name: "OIDCProvider does not exist and secret exists", storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) { *op = nil }, @@ -340,21 +343,17 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { }, secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) - secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + }, + wantOPActions: []kubetesting.Action{ + kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewCreateAction(secretGVR, namespace, goodSecret), }, }, - { - name: "OIDCProvider exists and valid secret exists", - secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { - secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) - secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(true) - secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1) - }, - }, { name: "OIDCProvider exists and invalid secret exists", storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) { @@ -363,7 +362,11 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(2).Return(false) - secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + }, + wantOPActions: []kubetesting.Action{ + kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), @@ -386,19 +389,23 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { secretHelper.EXPECT().Generate(goodOP).Times(1).Return(otherSecret, nil) secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(false) secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(true) - secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + }, + wantOPActions: []kubetesting.Action{ + kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), }, }, { - name: "OIDCProvider exists and invalid secret exists and get fails", + name: "OIDCProvider exists and invalid secret exists and getting secret fails", secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(1).Return(false) }, - client: func(c *kubernetesfake.Clientset) { + client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { c.PrependReactor("get", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { return true, nil, errors.New("some get error") }) @@ -409,14 +416,14 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { wantError: fmt.Sprintf("failed to create or update secret: failed to get secret %s/%s: some get error", namespace, goodSecret.Name), }, { - name: "OIDCProvider exists and secret does not exist and create fails", + name: "OIDCProvider exists and secret does not exist and creating secret fails", storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) { *s = nil }, secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) }, - client: func(c *kubernetesfake.Clientset) { + client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { c.PrependReactor("create", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { return true, nil, errors.New("some create error") }) @@ -428,12 +435,12 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { wantError: fmt.Sprintf("failed to create or update secret: failed to create secret %s/%s: some create error", namespace, goodSecret.Name), }, { - name: "OIDCProvider exists and invalid secret exists and update fails", + name: "OIDCProvider exists and invalid secret exists and updating secret fails", secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().IsValid(goodOP, goodSecret).Times(2).Return(false) }, - client: func(c *kubernetesfake.Clientset) { + client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { c.PrependReactor("update", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { return true, nil, errors.New("some update error") }) @@ -445,22 +452,27 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { wantError: "failed to create or update secret: some update error", }, { - name: "OIDCProvider exists and invalid secret exists and update fails due to conflict", + name: "OIDCProvider exists and invalid secret exists and updating secret fails due to conflict", storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) { *s = invalidSecret.DeepCopy() }, secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(3).Return(false) - secretHelper.EXPECT().Notify(goodOP, goodSecret).Times(1) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) }, - client: func(c *kubernetesfake.Clientset) { + client: func(_ *pinnipedfake.Clientset, c *kubernetesfake.Clientset) { + once := sync.Once{} c.PrependReactor("update", "secrets", func(_ kubetesting.Action) (bool, runtime.Object, error) { var err error once.Do(func() { err = k8serrors.NewConflict(secretGVR.GroupResource(), namespace, errors.New("some error")) }) return true, nil, err }) }, + wantOPActions: []kubetesting.Action{ + kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), + }, wantSecretActions: []kubetesting.Action{ kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), @@ -468,6 +480,59 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), }, }, + { + name: "OIDCProvider exists and invalid secret exists and getting OIDCProvider fails", + storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) { + *s = invalidSecret.DeepCopy() + }, + secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { + secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(2).Return(false) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + }, + client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { + c.PrependReactor("get", "oidcproviders", func(_ kubetesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("some get error") + }) + }, + wantOPActions: []kubetesting.Action{ + kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + }, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + wantError: fmt.Sprintf("failed to update oidcprovider: failed to get oidcprovider %s/%s: some get error", goodOPWithStatus.Namespace, goodOPWithStatus.Name), + }, + { + name: "OIDCProvider exists and invalid secret exists and updating OIDCProvider fails due to conflict", + storage: func(op **configv1alpha1.OIDCProvider, s **corev1.Secret) { + *s = invalidSecret.DeepCopy() + }, + secretHelper: func(secretHelper *mocksecrethelper.MockSecretHelper) { + secretHelper.EXPECT().Generate(goodOP).Times(1).Return(goodSecret, nil) + secretHelper.EXPECT().IsValid(goodOP, invalidSecret).Times(2).Return(false) + secretHelper.EXPECT().ObserveActiveSecretAndUpdateParentOIDCProvider(goodOP, goodSecret).Times(1).Return(goodOPWithStatus) + }, + client: func(c *pinnipedfake.Clientset, _ *kubernetesfake.Clientset) { + once := sync.Once{} + c.PrependReactor("update", "oidcproviders", func(_ kubetesting.Action) (bool, runtime.Object, error) { + var err error + once.Do(func() { err = k8serrors.NewConflict(secretGVR.GroupResource(), namespace, errors.New("some error")) }) + return true, nil, err + }) + }, + wantOPActions: []kubetesting.Action{ + kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), + kubetesting.NewGetAction(opGVR, namespace, goodOP.Name), + kubetesting.NewUpdateAction(opGVR, namespace, goodOPWithStatus), + }, + wantSecretActions: []kubetesting.Action{ + kubetesting.NewGetAction(secretGVR, namespace, goodSecret.Name), + kubetesting.NewUpdateAction(secretGVR, namespace, goodSecret), + }, + }, } for _, test := range tests { test := test @@ -477,6 +542,7 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) defer cancel() + pinnipedAPIClient := pinnipedfake.NewSimpleClientset() pinnipedInformerClient := pinnipedfake.NewSimpleClientset() kubeAPIClient := kubernetesfake.NewSimpleClientset() @@ -488,6 +554,7 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { test.storage(&op, &secret) } if op != nil { + require.NoError(t, pinnipedAPIClient.Tracker().Add(op)) require.NoError(t, pinnipedInformerClient.Tracker().Add(op)) } if secret != nil { @@ -496,7 +563,7 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { } if test.client != nil { - test.client(kubeAPIClient) + test.client(pinnipedAPIClient, kubeAPIClient) } kubeInformers := kubeinformers.NewSharedInformerFactory( @@ -519,6 +586,7 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { c := NewOIDCProviderSecretsController( secretHelper, kubeAPIClient, + pinnipedAPIClient, kubeInformers.Core().V1().Secrets(), pinnipedInformers.Config().V1alpha1().OIDCProviders(), controllerlib.WithInformer, @@ -542,6 +610,10 @@ func TestOIDCProviderSecretsControllerSync(t *testing.T) { } require.NoError(t, err) + if test.wantOPActions == nil { + test.wantOPActions = []kubetesting.Action{} + } + require.Equal(t, test.wantOPActions, pinnipedAPIClient.Actions()) if test.wantSecretActions == nil { test.wantSecretActions = []kubetesting.Action{} } diff --git a/internal/controller/supervisorconfig/generator/secret_helper.go b/internal/controller/supervisorconfig/generator/secret_helper.go index cfbcb5a7b..3a3362a66 100644 --- a/internal/controller/supervisorconfig/generator/secret_helper.go +++ b/internal/controller/supervisorconfig/generator/secret_helper.go @@ -12,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1" + "go.pinniped.dev/internal/plog" ) // SecretHelper describes an object that can Generate() a Secret and determine whether a Secret @@ -22,14 +23,14 @@ type SecretHelper interface { NamePrefix() string Generate(*configv1alpha1.OIDCProvider) (*corev1.Secret, error) IsValid(*configv1alpha1.OIDCProvider, *corev1.Secret) bool - Notify(*configv1alpha1.OIDCProvider, *corev1.Secret) + ObserveActiveSecretAndUpdateParentOIDCProvider(*configv1alpha1.OIDCProvider, *corev1.Secret) *configv1alpha1.OIDCProvider } const ( - // SymmetricSecretType is corev1.Secret.Type of all corev1.Secret's generated by this helper. - SymmetricSecretType = "secrets.pinniped.dev/symmetric" - // SymmetricSecretDataKey is the corev1.Secret.Data key for the symmetric key value generated by this helper. - SymmetricSecretDataKey = "key" + // symmetricSecretType is corev1.Secret.Type of all corev1.Secret's generated by this helper. + symmetricSecretType = "secrets.pinniped.dev/symmetric" + // symmetricSecretDataKey is the corev1.Secret.Data key for the symmetric key value generated by this helper. + symmetricSecretDataKey = "key" // symmetricKeySize is the default length, in bytes, of generated keys. It is set to 32 since this // seems like reasonable entropy for our keys, and a 32-byte key will allow for AES-256 @@ -37,18 +38,30 @@ const ( symmetricKeySize = 32 ) +// SecretUsage describes how a cryptographic secret is going to be used. It is currently used to +// indicate to a SecretHelper which status field to set on the parent OIDCProvider for a Secret. +type SecretUsage int + +const ( + SecretUsageTokenSigningKey SecretUsage = iota + SecretUsageStateSigningKey + SecretUsageStateEncryptionKey +) + // New returns a SecretHelper that has been parameterized with common symmetric secret generation // knobs. func NewSymmetricSecretHelper( namePrefix string, labels map[string]string, rand io.Reader, + secretUsage SecretUsage, updateCacheFunc func(cacheKey string, cacheValue []byte), ) SecretHelper { return &symmetricSecretHelper{ namePrefix: namePrefix, labels: labels, rand: rand, + secretUsage: secretUsage, updateCacheFunc: updateCacheFunc, } } @@ -57,6 +70,7 @@ type symmetricSecretHelper struct { namePrefix string labels map[string]string rand io.Reader + secretUsage SecretUsage updateCacheFunc func(cacheKey string, cacheValue []byte) } @@ -82,9 +96,9 @@ func (s *symmetricSecretHelper) Generate(parent *configv1alpha1.OIDCProvider) (* }), }, }, - Type: SymmetricSecretType, + Type: symmetricSecretType, Data: map[string][]byte{ - SymmetricSecretDataKey: key, + symmetricSecretDataKey: key, }, }, nil } @@ -95,11 +109,11 @@ func (s *symmetricSecretHelper) IsValid(parent *configv1alpha1.OIDCProvider, sec return false } - if secret.Type != SymmetricSecretType { + if secret.Type != symmetricSecretType { return false } - key, ok := secret.Data[SymmetricSecretDataKey] + key, ok := secret.Data[symmetricSecretDataKey] if !ok { return false } @@ -110,12 +124,28 @@ func (s *symmetricSecretHelper) IsValid(parent *configv1alpha1.OIDCProvider, sec return true } -// Notify implements SecretHelper.Notify(). -func (s *symmetricSecretHelper) Notify(op *configv1alpha1.OIDCProvider, secret *corev1.Secret) { +// ObserveActiveSecretAndUpdateParentOIDCProvider implements SecretHelper.ObserveActiveSecretAndUpdateParentOIDCProvider(). +func (s *symmetricSecretHelper) ObserveActiveSecretAndUpdateParentOIDCProvider( + op *configv1alpha1.OIDCProvider, + secret *corev1.Secret, +) *configv1alpha1.OIDCProvider { var cacheKey string if op != nil { cacheKey = op.Spec.Issuer } - s.updateCacheFunc(cacheKey, secret.Data[SymmetricSecretDataKey]) + s.updateCacheFunc(cacheKey, secret.Data[symmetricSecretDataKey]) + + switch s.secretUsage { + case SecretUsageTokenSigningKey: + op.Status.Secrets.TokenSigningKey.Name = secret.Name + case SecretUsageStateSigningKey: + op.Status.Secrets.StateSigningKey.Name = secret.Name + case SecretUsageStateEncryptionKey: + op.Status.Secrets.StateEncryptionKey.Name = secret.Name + default: + plog.Warning("unknown secret usage enum value: %d", s.secretUsage) + } + + return op } diff --git a/internal/controller/supervisorconfig/generator/secret_helper_test.go b/internal/controller/supervisorconfig/generator/secret_helper_test.go index ffe892416..bbaeb4e15 100644 --- a/internal/controller/supervisorconfig/generator/secret_helper_test.go +++ b/internal/controller/supervisorconfig/generator/secret_helper_test.go @@ -17,57 +17,98 @@ import ( const keyWith32Bytes = "0123456789abcdef0123456789abcdef" -func TestSymmetricSecretHHelper(t *testing.T) { - labels := map[string]string{ - "some-label-key-1": "some-label-value-1", - "some-label-key-2": "some-label-value-2", - } - randSource := strings.NewReader(keyWith32Bytes) - // var notifyParent *configv1alpha1.OIDCProvider - // var notifyChild *corev1.Secret - var oidcProviderIssuerValue string - var symmetricKeyValue []byte - h := NewSymmetricSecretHelper("some-name-prefix-", labels, randSource, func(oidcProviderIssuer string, symmetricKey []byte) { - require.True(t, oidcProviderIssuer == "" && symmetricKeyValue == nil, "expected notify func not to have been called yet") - oidcProviderIssuerValue = oidcProviderIssuer - symmetricKeyValue = symmetricKey - }) +func TestSymmetricSecretHelper(t *testing.T) { + t.Parallel() - parent := &configv1alpha1.OIDCProvider{ - ObjectMeta: metav1.ObjectMeta{ - UID: "some-uid", - Namespace: "some-namespace", - }, - } - child, err := h.Generate(parent) - require.NoError(t, err) - require.Equal(t, child, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-name-prefix-some-uid", - Namespace: "some-namespace", - Labels: labels, - OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(parent, schema.GroupVersionKind{ - Group: configv1alpha1.SchemeGroupVersion.Group, - Version: configv1alpha1.SchemeGroupVersion.Version, - Kind: "OIDCProvider", - }), + tests := []struct { + name string + secretUsage SecretUsage + wantSetOIDCProviderField func(*configv1alpha1.OIDCProvider) string + }{ + { + name: "token signing key", + secretUsage: SecretUsageTokenSigningKey, + wantSetOIDCProviderField: func(op *configv1alpha1.OIDCProvider) string { + return op.Status.Secrets.TokenSigningKey.Name }, }, - Type: "secrets.pinniped.dev/symmetric", - Data: map[string][]byte{ - "key": []byte(keyWith32Bytes), + { + name: "state signing key", + secretUsage: SecretUsageStateSigningKey, + wantSetOIDCProviderField: func(op *configv1alpha1.OIDCProvider) string { + return op.Status.Secrets.StateSigningKey.Name + }, }, - }) + { + name: "state encryption key", + secretUsage: SecretUsageStateEncryptionKey, + wantSetOIDCProviderField: func(op *configv1alpha1.OIDCProvider) string { + return op.Status.Secrets.StateEncryptionKey.Name + }, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() - require.True(t, h.IsValid(parent, child)) + labels := map[string]string{ + "some-label-key-1": "some-label-value-1", + "some-label-key-2": "some-label-value-2", + } + randSource := strings.NewReader(keyWith32Bytes) + var oidcProviderIssuerValue string + var symmetricKeyValue []byte + h := NewSymmetricSecretHelper( + "some-name-prefix-", + labels, + randSource, + test.secretUsage, + func(oidcProviderIssuer string, symmetricKey []byte) { + require.True(t, oidcProviderIssuer == "" && symmetricKeyValue == nil, "expected notify func not to have been called yet") + oidcProviderIssuerValue = oidcProviderIssuer + symmetricKeyValue = symmetricKey + }, + ) - h.Notify(parent, child) - require.Equal(t, parent.Spec.Issuer, oidcProviderIssuerValue) - require.Equal(t, child.Data[SymmetricSecretDataKey], symmetricKeyValue) + parent := &configv1alpha1.OIDCProvider{ + ObjectMeta: metav1.ObjectMeta{ + UID: "some-uid", + Namespace: "some-namespace", + }, + } + child, err := h.Generate(parent) + require.NoError(t, err) + require.Equal(t, child, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name-prefix-some-uid", + Namespace: "some-namespace", + Labels: labels, + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(parent, schema.GroupVersionKind{ + Group: configv1alpha1.SchemeGroupVersion.Group, + Version: configv1alpha1.SchemeGroupVersion.Version, + Kind: "OIDCProvider", + }), + }, + }, + Type: "secrets.pinniped.dev/symmetric", + Data: map[string][]byte{ + "key": []byte(keyWith32Bytes), + }, + }) + + require.True(t, h.IsValid(parent, child)) + + h.ObserveActiveSecretAndUpdateParentOIDCProvider(parent, child) + require.Equal(t, parent.Spec.Issuer, oidcProviderIssuerValue) + require.Equal(t, child.Name, test.wantSetOIDCProviderField(parent)) + require.Equal(t, child.Data["key"], symmetricKeyValue) + }) + } } -func TestSymmetricSecretHHelperIsValid(t *testing.T) { +func TestSymmetricSecretHelperIsValid(t *testing.T) { tests := []struct { name string child func(*corev1.Secret) @@ -117,7 +158,7 @@ func TestSymmetricSecretHHelperIsValid(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - h := NewSymmetricSecretHelper("none of these args matter", nil, nil, nil) + h := NewSymmetricSecretHelper("none of these args matter", nil, nil, SecretUsageTokenSigningKey, nil) parent := &configv1alpha1.OIDCProvider{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controller/supervisorconfig/generator/supervisor_secrets.go b/internal/controller/supervisorconfig/generator/supervisor_secrets.go index ec89993fc..d3155520a 100644 --- a/internal/controller/supervisorconfig/generator/supervisor_secrets.go +++ b/internal/controller/supervisorconfig/generator/supervisor_secrets.go @@ -79,7 +79,7 @@ func (c *supervisorSecretsController) Sync(ctx controllerlib.Context) error { secretNeedsUpdate := isNotFound || !isValid(secret) if !secretNeedsUpdate { plog.Debug("secret is up to date", "secret", klog.KObj(secret)) - c.setCacheFunc(secret.Data[SymmetricSecretDataKey]) + c.setCacheFunc(secret.Data[symmetricSecretDataKey]) return nil } @@ -97,7 +97,7 @@ func (c *supervisorSecretsController) Sync(ctx controllerlib.Context) error { return fmt.Errorf("failed to create/update secret %s/%s: %w", newSecret.Namespace, newSecret.Name, err) } - c.setCacheFunc(newSecret.Data[SymmetricSecretDataKey]) + c.setCacheFunc(newSecret.Data[symmetricSecretDataKey]) return nil } diff --git a/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go b/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go index c5357ce16..307e1f669 100644 --- a/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go +++ b/internal/controller/supervisorconfig/generator/supervisor_secrets_test.go @@ -194,8 +194,8 @@ func TestSupervisorSecretsControllerInitialEvent(t *testing.T) { initialEventOption.WithInitialEvent, ) require.Equal(t, &controllerlib.Key{ - owner.Namespace, - owner.Name + "-key", + Namespace: owner.Namespace, + Name: owner.Name + "-key", }, initialEventOption.GetInitialEventKey()) } diff --git a/internal/controller/supervisorconfig/jwks_observer.go b/internal/controller/supervisorconfig/jwks_observer.go index 5c01c5b46..efbdfae92 100644 --- a/internal/controller/supervisorconfig/jwks_observer.go +++ b/internal/controller/supervisorconfig/jwks_observer.go @@ -76,7 +76,7 @@ func (c *jwksObserverController) Sync(ctx controllerlib.Context) error { issuerToActiveJWKMap := map[string]*jose.JSONWebKey{} for _, provider := range allProviders { - secretRef := provider.Status.JWKSSecret + secretRef := provider.Status.Secrets.JWKS jwksSecret, err := c.secretInformer.Lister().Secrets(ns).Get(secretRef.Name) if err != nil { plog.Debug("jwksObserverController Sync could not find JWKS secret", "namespace", ns, "secretName", secretRef.Name) diff --git a/internal/controller/supervisorconfig/jwks_observer_test.go b/internal/controller/supervisorconfig/jwks_observer_test.go index ad7323b80..fa2ebea1b 100644 --- a/internal/controller/supervisorconfig/jwks_observer_test.go +++ b/internal/controller/supervisorconfig/jwks_observer_test.go @@ -202,7 +202,7 @@ func TestJWKSObserverControllerSync(t *testing.T) { Namespace: installedInNamespace, }, Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://no-secret-issuer1.com"}, - Status: v1alpha1.OIDCProviderStatus{}, // no JWKSSecret field + Status: v1alpha1.OIDCProviderStatus{}, // no Secrets.JWKS field } oidcProviderWithoutSecret2 := &v1alpha1.OIDCProvider{ ObjectMeta: metav1.ObjectMeta{ @@ -219,7 +219,9 @@ func TestJWKSObserverControllerSync(t *testing.T) { }, Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://bad-secret-issuer.com"}, Status: v1alpha1.OIDCProviderStatus{ - JWKSSecret: corev1.LocalObjectReference{Name: "bad-secret-name"}, + Secrets: v1alpha1.OIDCProviderSecrets{ + JWKS: corev1.LocalObjectReference{Name: "bad-secret-name"}, + }, }, } oidcProviderWithBadJWKSSecret := &v1alpha1.OIDCProvider{ @@ -229,7 +231,9 @@ func TestJWKSObserverControllerSync(t *testing.T) { }, Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://bad-jwks-secret-issuer.com"}, Status: v1alpha1.OIDCProviderStatus{ - JWKSSecret: corev1.LocalObjectReference{Name: "bad-jwks-secret-name"}, + Secrets: v1alpha1.OIDCProviderSecrets{ + JWKS: corev1.LocalObjectReference{Name: "bad-jwks-secret-name"}, + }, }, } oidcProviderWithBadActiveJWKSecret := &v1alpha1.OIDCProvider{ @@ -239,7 +243,9 @@ func TestJWKSObserverControllerSync(t *testing.T) { }, Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://bad-active-jwk-secret-issuer.com"}, Status: v1alpha1.OIDCProviderStatus{ - JWKSSecret: corev1.LocalObjectReference{Name: "bad-active-jwk-secret-name"}, + Secrets: v1alpha1.OIDCProviderSecrets{ + JWKS: corev1.LocalObjectReference{Name: "bad-active-jwk-secret-name"}, + }, }, } oidcProviderWithGoodSecret1 := &v1alpha1.OIDCProvider{ @@ -249,7 +255,9 @@ func TestJWKSObserverControllerSync(t *testing.T) { }, Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://issuer-with-good-secret1.com"}, Status: v1alpha1.OIDCProviderStatus{ - JWKSSecret: corev1.LocalObjectReference{Name: "good-jwks-secret-name1"}, + Secrets: v1alpha1.OIDCProviderSecrets{ + JWKS: corev1.LocalObjectReference{Name: "good-jwks-secret-name1"}, + }, }, } oidcProviderWithGoodSecret2 := &v1alpha1.OIDCProvider{ @@ -259,7 +267,9 @@ func TestJWKSObserverControllerSync(t *testing.T) { }, Spec: v1alpha1.OIDCProviderSpec{Issuer: "https://issuer-with-good-secret2.com"}, Status: v1alpha1.OIDCProviderStatus{ - JWKSSecret: corev1.LocalObjectReference{Name: "good-jwks-secret-name2"}, + Secrets: v1alpha1.OIDCProviderSecrets{ + JWKS: corev1.LocalObjectReference{Name: "good-jwks-secret-name2"}, + }, }, } expectedJWK1 = string(readJWKJSON(t, "testdata/public-jwk.json")) diff --git a/internal/controller/supervisorconfig/jwks_writer.go b/internal/controller/supervisorconfig/jwks_writer.go index 4e6a51a13..2a33b77f6 100644 --- a/internal/controller/supervisorconfig/jwks_writer.go +++ b/internal/controller/supervisorconfig/jwks_writer.go @@ -169,7 +169,7 @@ func (c *jwksWriterController) Sync(ctx controllerlib.Context) error { // Ensure that the OPC points to the secret. newOPC := opc.DeepCopy() - newOPC.Status.JWKSSecret.Name = secret.Name + newOPC.Status.Secrets.JWKS.Name = secret.Name if err := c.updateOPC(ctx.Context, newOPC); err != nil { return fmt.Errorf("cannot update opc: %w", err) } @@ -179,13 +179,13 @@ func (c *jwksWriterController) Sync(ctx controllerlib.Context) error { } func (c *jwksWriterController) secretNeedsUpdate(opc *configv1alpha1.OIDCProvider) (bool, error) { - if opc.Status.JWKSSecret.Name == "" { + if opc.Status.Secrets.JWKS.Name == "" { // If the OPC says it doesn't have a secret associated with it, then let's create one. return true, nil } // This OPC says it has a secret associated with it. Let's try to get it from the cache. - secret, err := c.secretInformer.Lister().Secrets(opc.Namespace).Get(opc.Status.JWKSSecret.Name) + secret, err := c.secretInformer.Lister().Secrets(opc.Namespace).Get(opc.Status.Secrets.JWKS.Name) notFound := k8serrors.IsNotFound(err) if err != nil && !notFound { return false, fmt.Errorf("cannot get secret: %w", err) @@ -301,12 +301,12 @@ func (c *jwksWriterController) updateOPC( return fmt.Errorf("cannot get opc: %w", err) } - if newOPC.Status.JWKSSecret.Name == oldOPC.Status.JWKSSecret.Name { + if newOPC.Status.Secrets.JWKS.Name == oldOPC.Status.Secrets.JWKS.Name { // If the existing OPC is up to date, we don't need to update it. return nil } - oldOPC.Status.JWKSSecret.Name = newOPC.Status.JWKSSecret.Name + oldOPC.Status.Secrets.JWKS.Name = newOPC.Status.Secrets.JWKS.Name _, err = opcClient.Update(ctx, oldOPC, metav1.UpdateOptions{}) return err }) diff --git a/internal/controller/supervisorconfig/jwks_writer_test.go b/internal/controller/supervisorconfig/jwks_writer_test.go index 9afdc4861..615a1f3d7 100644 --- a/internal/controller/supervisorconfig/jwks_writer_test.go +++ b/internal/controller/supervisorconfig/jwks_writer_test.go @@ -253,7 +253,7 @@ func TestJWKSWriterControllerSync(t *testing.T) { }, } goodOPCWithStatus := goodOPC.DeepCopy() - goodOPCWithStatus.Status.JWKSSecret.Name = goodOPCWithStatus.Name + "-jwks" + goodOPCWithStatus.Status.Secrets.JWKS.Name = goodOPCWithStatus.Name + "-jwks" secretGVR := schema.GroupVersionResource{ Group: corev1.SchemeGroupVersion.Group, @@ -264,7 +264,7 @@ func TestJWKSWriterControllerSync(t *testing.T) { newSecret := func(activeJWKPath, jwksPath string) *corev1.Secret { s := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: goodOPCWithStatus.Status.JWKSSecret.Name, + Name: goodOPCWithStatus.Status.Secrets.JWKS.Name, Namespace: namespace, Labels: map[string]string{ "myLabelKey1": "myLabelValue1", diff --git a/internal/mocks/mocksecrethelper/mocksecrethelper.go b/internal/mocks/mocksecrethelper/mocksecrethelper.go index d191d2d98..68b6cfbdc 100644 --- a/internal/mocks/mocksecrethelper/mocksecrethelper.go +++ b/internal/mocks/mocksecrethelper/mocksecrethelper.go @@ -81,14 +81,16 @@ func (mr *MockSecretHelperMockRecorder) NamePrefix() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NamePrefix", reflect.TypeOf((*MockSecretHelper)(nil).NamePrefix)) } -// Notify mocks base method -func (m *MockSecretHelper) Notify(arg0 *v1alpha1.OIDCProvider, arg1 *v1.Secret) { +// ObserveActiveSecretAndUpdateParentOIDCProvider mocks base method +func (m *MockSecretHelper) ObserveActiveSecretAndUpdateParentOIDCProvider(arg0 *v1alpha1.OIDCProvider, arg1 *v1.Secret) *v1alpha1.OIDCProvider { m.ctrl.T.Helper() - m.ctrl.Call(m, "Notify", arg0, arg1) + ret := m.ctrl.Call(m, "ObserveActiveSecretAndUpdateParentOIDCProvider", arg0, arg1) + ret0, _ := ret[0].(*v1alpha1.OIDCProvider) + return ret0 } -// Notify indicates an expected call of Notify -func (mr *MockSecretHelperMockRecorder) Notify(arg0, arg1 interface{}) *gomock.Call { +// ObserveActiveSecretAndUpdateParentOIDCProvider indicates an expected call of ObserveActiveSecretAndUpdateParentOIDCProvider +func (mr *MockSecretHelperMockRecorder) ObserveActiveSecretAndUpdateParentOIDCProvider(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Notify", reflect.TypeOf((*MockSecretHelper)(nil).Notify), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ObserveActiveSecretAndUpdateParentOIDCProvider", reflect.TypeOf((*MockSecretHelper)(nil).ObserveActiveSecretAndUpdateParentOIDCProvider), arg0, arg1) } diff --git a/test/integration/supervisor_secrets_test.go b/test/integration/supervisor_secrets_test.go index 9c3f8c0f1..038b6fac9 100644 --- a/test/integration/supervisor_secrets_test.go +++ b/test/integration/supervisor_secrets_test.go @@ -45,28 +45,28 @@ func TestSupervisorSecrets(t *testing.T) { { name: "jwks", secretName: func(op *configv1alpha1.OIDCProvider) string { - return op.Status.JWKSSecret.Name + return op.Status.Secrets.JWKS.Name }, ensureValid: ensureValidJWKS, }, { name: "hmac signing secret", secretName: func(op *configv1alpha1.OIDCProvider) string { - return "pinniped-oidc-provider-hmac-key-" + string(op.UID) + return op.Status.Secrets.TokenSigningKey.Name }, ensureValid: ensureValidSymmetricKey, }, { name: "state signature secret", secretName: func(op *configv1alpha1.OIDCProvider) string { - return "pinniped-oidc-provider-upstream-state-signature-key-" + string(op.UID) + return op.Status.Secrets.StateSigningKey.Name }, ensureValid: ensureValidSymmetricKey, }, { name: "state encryption secret", secretName: func(op *configv1alpha1.OIDCProvider) string { - return "pinniped-oidc-provider-upstream-state-encryption-key-" + string(op.UID) + return op.Status.Secrets.StateEncryptionKey.Name }, ensureValid: ensureValidSymmetricKey, },