From 245d5f0b7b2719ee4b6b22fac416f4ead4adbff4 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Tue, 30 Jul 2024 09:18:56 -0700 Subject: [PATCH] feat: fixes RHOAIENG-10035: add env variable DEFAULT_CERT to specify default cert secret name in upstream odh mr operator (#119) --- api/v1alpha1/modelregistry_types.go | 1 + cmd/main.go | 3 + ...gistry.opendatahub.io_modelregistries.yaml | 56 ++++++++++--------- config/manager/manager.yaml | 2 + config/overlays/odh/params.env | 3 +- config/overlays/odh/replacements.yaml | 10 ++++ .../controller/modelregistry_controller.go | 25 ++++++++- 7 files changed, 70 insertions(+), 30 deletions(-) diff --git a/api/v1alpha1/modelregistry_types.go b/api/v1alpha1/modelregistry_types.go index 70f21d5..724d625 100644 --- a/api/v1alpha1/modelregistry_types.go +++ b/api/v1alpha1/modelregistry_types.go @@ -189,6 +189,7 @@ type TLSServerSettings struct { Mode string `json:"mode"` // The name of the secret that holds the TLS certs including the CA certificates. + // If not provided, it is set automatically using model registry operator env variable DEFAULT_CERT. // An Opaque secret should contain the following // keys and values: `tls.key: ` and `tls.crt: ` or // `key: ` and `cert: `. diff --git a/cmd/main.go b/cmd/main.go index fc021f7..ffe7c31 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -55,6 +55,7 @@ const ( EnableWebhooks = "ENABLE_WEBHOOKS" CreateAuthResources = "CREATE_AUTH_RESOURCES" DefaultDomain = "DEFAULT_DOMAIN" + DefaultCert = "DEFAULT_CERT" ) func init() { @@ -158,6 +159,7 @@ func main() { enableWebhooks := os.Getenv(EnableWebhooks) != "false" createAuthResources := os.Getenv(CreateAuthResources) != "false" defaultDomain := os.Getenv(DefaultDomain) + defaultCert := os.Getenv(DefaultCert) if err = (&controller.ModelRegistryReconciler{ Client: client, @@ -171,6 +173,7 @@ func main() { Audiences: tokenReview.Status.Audiences, CreateAuthResources: createAuthResources, DefaultDomain: defaultDomain, + DefaultCert: defaultCert, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ModelRegistry") os.Exit(1) diff --git a/config/crd/bases/modelregistry.opendatahub.io_modelregistries.yaml b/config/crd/bases/modelregistry.opendatahub.io_modelregistries.yaml index 77e46b7..b1db282 100644 --- a/config/crd/bases/modelregistry.opendatahub.io_modelregistries.yaml +++ b/config/crd/bases/modelregistry.opendatahub.io_modelregistries.yaml @@ -192,19 +192,21 @@ spec: properties: credentialName: description: 'The name of the secret that holds the - TLS certs including the CA certificates. An Opaque - secret should contain the following keys and values: - `tls.key: ` and `tls.crt: ` - or `key: ` and `cert: `. - For mutual TLS, `cacert: ` and `crl: - ` can be provided in - the same secret or a separate secret named `-cacert`. - A TLS secret for server certificates with an additional - `tls.ocsp-staple` key for specifying OCSP staple - information, `ca.crt` key for CA certificates and - `ca.crl` for certificate revocation list is also - supported. Only one of server certificates and CA - certificate or credentialName can be specified.' + TLS certs including the CA certificates. If not + provided, it is set automatically using model registry + operator env variable DEFAULT_CERT. An Opaque secret + should contain the following keys and values: `tls.key: + ` and `tls.crt: ` or `key: + ` and `cert: `. For mutual + TLS, `cacert: ` and `crl: ` + can be provided in the same secret or a separate + secret named `-cacert`. A TLS secret for + server certificates with an additional `tls.ocsp-staple` + key for specifying OCSP staple information, `ca.crt` + key for CA certificates and `ca.crl` for certificate + revocation list is also supported. Only one of server + certificates and CA certificate or credentialName + can be specified.' type: string mode: default: SIMPLE @@ -271,19 +273,21 @@ spec: properties: credentialName: description: 'The name of the secret that holds the - TLS certs including the CA certificates. An Opaque - secret should contain the following keys and values: - `tls.key: ` and `tls.crt: ` - or `key: ` and `cert: `. - For mutual TLS, `cacert: ` and `crl: - ` can be provided in - the same secret or a separate secret named `-cacert`. - A TLS secret for server certificates with an additional - `tls.ocsp-staple` key for specifying OCSP staple - information, `ca.crt` key for CA certificates and - `ca.crl` for certificate revocation list is also - supported. Only one of server certificates and CA - certificate or credentialName can be specified.' + TLS certs including the CA certificates. If not + provided, it is set automatically using model registry + operator env variable DEFAULT_CERT. An Opaque secret + should contain the following keys and values: `tls.key: + ` and `tls.crt: ` or `key: + ` and `cert: `. For mutual + TLS, `cacert: ` and `crl: ` + can be provided in the same secret or a separate + secret named `-cacert`. A TLS secret for + server certificates with an additional `tls.ocsp-staple` + key for specifying OCSP staple information, `ca.crt` + key for CA certificates and `ca.crl` for certificate + revocation list is also supported. Only one of server + certificates and CA certificate or credentialName + can be specified.' type: string mode: default: SIMPLE diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 1352bd8..15e6d7c 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -83,6 +83,8 @@ spec: value: "true" - name: DEFAULT_DOMAIN value: "" + - name: DEFAULT_CERT + value: "" securityContext: allowPrivilegeEscalation: false capabilities: diff --git a/config/overlays/odh/params.env b/config/overlays/odh/params.env index 782396d..2aa4030 100644 --- a/config/overlays/odh/params.env +++ b/config/overlays/odh/params.env @@ -1,4 +1,5 @@ IMAGES_MODELREGISTRY_OPERATOR=quay.io/opendatahub/model-registry-operator:latest IMAGES_GRPC_SERVICE=quay.io/opendatahub/mlmd-grpc-server:latest IMAGES_REST_SERVICE=quay.io/opendatahub/model-registry:latest -CREATE_AUTH_RESOURCES=true \ No newline at end of file +CREATE_AUTH_RESOURCES=true +DEFAULT_CERT= \ No newline at end of file diff --git a/config/overlays/odh/replacements.yaml b/config/overlays/odh/replacements.yaml index 01cb78e..e0292f9 100644 --- a/config/overlays/odh/replacements.yaml +++ b/config/overlays/odh/replacements.yaml @@ -39,3 +39,13 @@ name: controller-manager fieldPaths: - spec.template.spec.containers.[name=manager].env.[name=CREATE_AUTH_RESOURCES].value +- source: + kind: ConfigMap + name: model-registry-operator-parameters + fieldPath: data.DEFAULT_CERT + targets: + - select: + kind: Deployment + name: controller-manager + fieldPaths: + - spec.template.spec.containers.[name=manager].env.[name=DEFAULT_CERT].value diff --git a/internal/controller/modelregistry_controller.go b/internal/controller/modelregistry_controller.go index c2542c8..d2762ae 100644 --- a/internal/controller/modelregistry_controller.go +++ b/internal/controller/modelregistry_controller.go @@ -71,6 +71,7 @@ type ModelRegistryReconciler struct { Audiences []string CreateAuthResources bool DefaultDomain string + DefaultCert string } // Reconcile is part of the main kubernetes reconciliation loop which aims to @@ -604,16 +605,20 @@ func (r *ModelRegistryReconciler) createOrUpdateGateway(ctx context.Context, par result = ResourceUnchanged + updated := false // autoconfigure domain if needed - domainUpdated := false if len(registry.Spec.Istio.Gateway.Domain) == 0 { err = r.setClusterDomain(ctx, registry) if err != nil { return result, err } + updated = true + } + // set default cert if needed + updated = updated || r.setDefaultCert(registry) + if updated { // update current reconcile spec params.Spec = registry.Spec - domainUpdated = true } var gateway networking.Gateway @@ -629,7 +634,7 @@ func (r *ModelRegistryReconciler) createOrUpdateGateway(ctx context.Context, par return result, err } - if !domainUpdated { + if !updated { return result, nil } else { return ResourceUpdated, nil @@ -1005,3 +1010,17 @@ func (r *ModelRegistryReconciler) setClusterDomain(ctx context.Context, registry // update domain in model registry resource return r.Client.Update(ctx, registry) } + +func (r *ModelRegistryReconciler) setDefaultCert(registry *modelregistryv1alpha1.ModelRegistry) bool { + updated := false + gateway := registry.Spec.Istio.Gateway + if gateway.Rest.TLS != nil && gateway.Rest.TLS.Mode != "ISTIO_MUTUAL" && gateway.Rest.TLS.CredentialName == nil { + gateway.Rest.TLS.CredentialName = &r.DefaultCert + updated = true + } + if gateway.Grpc.TLS != nil && gateway.Grpc.TLS.Mode != "ISTIO_MUTUAL" && gateway.Grpc.TLS.CredentialName == nil { + gateway.Grpc.TLS.CredentialName = &r.DefaultCert + updated = true + } + return updated +}