Skip to content

Commit

Permalink
feat: fixes RHOAIENG-10035: add env variable DEFAULT_CERT to specify …
Browse files Browse the repository at this point in the history
…default cert secret name in upstream odh mr operator (#119)
  • Loading branch information
dhirajsb authored Jul 30, 2024
1 parent 15a9698 commit 245d5f0
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 30 deletions.
1 change: 1 addition & 0 deletions api/v1alpha1/modelregistry_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: <privateKey>` and `tls.crt: <serverCert>` or
// `key: <privateKey>` and `cert: <serverCert>`.
Expand Down
3 changes: 3 additions & 0 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const (
EnableWebhooks = "ENABLE_WEBHOOKS"
CreateAuthResources = "CREATE_AUTH_RESOURCES"
DefaultDomain = "DEFAULT_DOMAIN"
DefaultCert = "DEFAULT_CERT"
)

func init() {
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down
56 changes: 30 additions & 26 deletions config/crd/bases/modelregistry.opendatahub.io_modelregistries.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: <privateKey>` and `tls.crt: <serverCert>`
or `key: <privateKey>` and `cert: <serverCert>`.
For mutual TLS, `cacert: <CACertificate>` and `crl:
<CertificateRevocationList>` can be provided in
the same secret or a separate secret named `<secret>-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:
<privateKey>` and `tls.crt: <serverCert>` or `key:
<privateKey>` and `cert: <serverCert>`. For mutual
TLS, `cacert: <CACertificate>` and `crl: <CertificateRevocationList>`
can be provided in the same secret or a separate
secret named `<secret>-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
Expand Down Expand Up @@ -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: <privateKey>` and `tls.crt: <serverCert>`
or `key: <privateKey>` and `cert: <serverCert>`.
For mutual TLS, `cacert: <CACertificate>` and `crl:
<CertificateRevocationList>` can be provided in
the same secret or a separate secret named `<secret>-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:
<privateKey>` and `tls.crt: <serverCert>` or `key:
<privateKey>` and `cert: <serverCert>`. For mutual
TLS, `cacert: <CACertificate>` and `crl: <CertificateRevocationList>`
can be provided in the same secret or a separate
secret named `<secret>-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
Expand Down
2 changes: 2 additions & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ spec:
value: "true"
- name: DEFAULT_DOMAIN
value: ""
- name: DEFAULT_CERT
value: ""
securityContext:
allowPrivilegeEscalation: false
capabilities:
Expand Down
3 changes: 2 additions & 1 deletion config/overlays/odh/params.env
Original file line number Diff line number Diff line change
@@ -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
CREATE_AUTH_RESOURCES=true
DEFAULT_CERT=
10 changes: 10 additions & 0 deletions config/overlays/odh/replacements.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
25 changes: 22 additions & 3 deletions internal/controller/modelregistry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}

0 comments on commit 245d5f0

Please sign in to comment.