From d8e33c1a6e3c6937dc58381684efb835bf578197 Mon Sep 17 00:00:00 2001 From: Vaishnavi Hire Date: Wed, 19 Jun 2024 07:33:43 -0400 Subject: [PATCH] Add support for default Ingress Cert (#1022) * Add support for default ingress cert * Refactor watches * Add Watch for Ingress cert secret * Update manifests * Fix linters and api docs * Update scheme and roles * Address comments * Update tests to match default values * chore: refactors cert creation it does not have to be part of Feature struct/methods Signed-off-by: bartoszmajsak * Change default value to OpenshiftDefaultIngress * Update default secret * Add e2e tests * Update tests for Managed Serving * Update serverless tests * Sync manifests --- README.md | 2 +- apis/infrastructure/v1/cert_types.go | 10 +- ...er.opendatahub.io_datascienceclusters.yaml | 7 +- .../rhods-operator.clusterserviceversion.yaml | 12 +- ...er.opendatahub.io_datascienceclusters.yaml | 7 +- config/rbac/role.yaml | 9 + ...asciencecluster_v1_datasciencecluster.yaml | 2 +- .../datasciencecluster_controller.go | 82 +++++-- .../datasciencecluster/kubebuilder_rbac.go | 3 +- docs/api-overview.md | 2 +- main.go | 8 +- pkg/cluster/cert.go | 225 ++++++++++++++++++ pkg/feature/cert.go | 128 ---------- pkg/feature/serverless/loaders.go | 4 +- pkg/feature/serverless/resources.go | 17 +- tests/e2e/controller_setup_test.go | 2 + tests/e2e/dsc_creation_test.go | 43 +++- tests/e2e/helper_test.go | 2 +- .../features/serverless_feature_test.go | 5 +- 19 files changed, 393 insertions(+), 177 deletions(-) create mode 100644 pkg/cluster/cert.go delete mode 100644 pkg/feature/cert.go diff --git a/README.md b/README.md index 39f4d155e45..4b22da10723 100644 --- a/README.md +++ b/README.md @@ -304,7 +304,7 @@ spec: serving: ingressGateway: certificate: - type: SelfSigned + type: OpenshiftDefaultIngress managementState: Managed name: knative-serving kueue: diff --git a/apis/infrastructure/v1/cert_types.go b/apis/infrastructure/v1/cert_types.go index 6843d7a3c04..345d7e9d0ea 100644 --- a/apis/infrastructure/v1/cert_types.go +++ b/apis/infrastructure/v1/cert_types.go @@ -3,8 +3,9 @@ package v1 type CertType string const ( - SelfSigned CertType = "SelfSigned" - Provided CertType = "Provided" + SelfSigned CertType = "SelfSigned" + Provided CertType = "Provided" + OpenshiftDefaultIngress CertType = "OpenshiftDefaultIngress" ) // CertificateSpec represents the specification of the certificate securing communications of @@ -17,7 +18,8 @@ type CertificateSpec struct { // is provided by the user. Allowed values are: // * SelfSigned: A certificate is going to be generated using an own private key. // * Provided: Pre-existence of the TLS Secret (see SecretName) with a valid certificate is assumed. - // +kubebuilder:validation:Enum=SelfSigned;Provided - // +kubebuilder:default=SelfSigned + // * OpenshiftDefaultIngress: Default ingress certificate configured for OpenShift + // +kubebuilder:validation:Enum=SelfSigned;Provided;OpenshiftDefaultIngress + // +kubebuilder:default=OpenshiftDefaultIngress Type CertType `json:"type,omitempty"` } diff --git a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 7cc31ac77f7..e2c36fe8ae9 100644 --- a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -254,17 +254,20 @@ spec: for the KNative network. type: string type: - default: SelfSigned + default: OpenshiftDefaultIngress description: 'Type specifies if the TLS certificate should be generated automatically, or if the certificate is provided by the user. Allowed values are: * SelfSigned: A certificate is going to be generated using an own private key. * Provided: Pre-existence of the TLS Secret (see - SecretName) with a valid certificate is assumed.' + SecretName) with a valid certificate is assumed. + * OpenshiftDefaultIngress: Default ingress certificate + configured for OpenShift' enum: - SelfSigned - Provided + - OpenshiftDefaultIngress type: string type: object domain: diff --git a/bundle/manifests/rhods-operator.clusterserviceversion.yaml b/bundle/manifests/rhods-operator.clusterserviceversion.yaml index 9611d5c8e48..bba8e8098e9 100644 --- a/bundle/manifests/rhods-operator.clusterserviceversion.yaml +++ b/bundle/manifests/rhods-operator.clusterserviceversion.yaml @@ -33,7 +33,7 @@ metadata: "serving": { "ingressGateway": { "certificate": { - "type": "SelfSigned" + "type": "OpenshiftDefaultIngress" } }, "managementState": "Managed", @@ -1298,6 +1298,16 @@ spec: - list - patch - watch + - apiGroups: + - operator.openshift.io + resources: + - ingresscontrollers + verbs: + - delete + - get + - list + - patch + - watch - apiGroups: - operators.coreos.com resources: diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index d99184cc4ce..978f776eb7f 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -255,17 +255,20 @@ spec: for the KNative network. type: string type: - default: SelfSigned + default: OpenshiftDefaultIngress description: 'Type specifies if the TLS certificate should be generated automatically, or if the certificate is provided by the user. Allowed values are: * SelfSigned: A certificate is going to be generated using an own private key. * Provided: Pre-existence of the TLS Secret (see - SecretName) with a valid certificate is assumed.' + SecretName) with a valid certificate is assumed. + * OpenshiftDefaultIngress: Default ingress certificate + configured for OpenShift' enum: - SelfSigned - Provided + - OpenshiftDefaultIngress type: string type: object domain: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index d765446bb6c..45980035290 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -1053,6 +1053,15 @@ rules: - consoles verbs: - delete + - list + - patch + - watch +- apiGroups: + - operator.openshift.io + resources: + - ingresscontrollers + verbs: + - delete - get - list - patch diff --git a/config/samples/datasciencecluster_v1_datasciencecluster.yaml b/config/samples/datasciencecluster_v1_datasciencecluster.yaml index d230ba15fc8..ab6c4c2f4df 100644 --- a/config/samples/datasciencecluster_v1_datasciencecluster.yaml +++ b/config/samples/datasciencecluster_v1_datasciencecluster.yaml @@ -21,7 +21,7 @@ spec: serving: { ingressGateway: { certificate: { - type: SelfSigned + type: OpenshiftDefaultIngress } }, name: "knative-serving", diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go index ea139bb7e2d..0f46f08851c 100644 --- a/controllers/datasciencecluster/datasciencecluster_controller.go +++ b/controllers/datasciencecluster/datasciencecluster_controller.go @@ -472,26 +472,17 @@ func (r *DataScienceClusterReconciler) SetupWithManager(mgr ctrl.Manager) error Watches(&source.Kind{Type: &corev1.ConfigMap{}}, handler.EnqueueRequestsFromMapFunc(r.watchDataScienceClusterResources), builder.WithPredicates(configMapPredicates)). Watches(&source.Kind{Type: &apiextensionsv1.CustomResourceDefinition{}}, handler.EnqueueRequestsFromMapFunc(r.watchDataScienceClusterResources), builder.WithPredicates(argoWorkflowCRDPredicates)). + Watches(&source.Kind{Type: &corev1.Secret{}}, handler.EnqueueRequestsFromMapFunc(r.watchDefaultIngressSecret), builder.WithPredicates(defaultIngressCertSecretPredicates)). // this predicates prevents meaningless reconciliations from being triggered WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, predicate.LabelChangedPredicate{})). Complete(r) } func (r *DataScienceClusterReconciler) watchDataScienceClusterForDSCI(a client.Object) []reconcile.Request { - instanceList := &dsc.DataScienceClusterList{} - err := r.Client.List(context.TODO(), instanceList) + requestName, err := r.getRequestName() if err != nil { return nil } - var requestName string - switch { - case len(instanceList.Items) == 1: - requestName = instanceList.Items[0].Name - case len(instanceList.Items) == 0: - requestName = "default-dsc" - default: - return nil - } // When DSCI CR gets created, trigger reconcile function if a.GetObjectKind().GroupVersionKind().Kind == "DSCInitialization" || a.GetName() == "default-dsci" { return []reconcile.Request{{ @@ -501,19 +492,15 @@ func (r *DataScienceClusterReconciler) watchDataScienceClusterForDSCI(a client.O return nil } func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(a client.Object) []reconcile.Request { - instanceList := &dsc.DataScienceClusterList{} - err := r.Client.List(context.TODO(), instanceList) + requestName, err := r.getRequestName() if err != nil { return nil } - var requestName string - switch { - case len(instanceList.Items) == 1: - requestName = instanceList.Items[0].Name - case len(instanceList.Items) == 0: - requestName = "default-dsc" - default: - return nil + + if a.GetObjectKind().GroupVersionKind().Kind == "CustomResourceDefinition" || a.GetName() == "ArgoWorkflowCRD" { + return []reconcile.Request{{ + NamespacedName: types.NamespacedName{Name: requestName}, + }} } if a.GetObjectKind().GroupVersionKind().Kind == "CustomResourceDefinition" { @@ -528,17 +515,33 @@ func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(a client return nil } if a.GetNamespace() == operatorNs { - labels := a.GetLabels() - if val, ok := labels[upgrade.DeleteConfigMapLabel]; ok && val == "true" { + cmLabels := a.GetLabels() + if val, ok := cmLabels[upgrade.DeleteConfigMapLabel]; ok && val == "true" { return []reconcile.Request{{ NamespacedName: types.NamespacedName{Name: requestName}, }} } - return nil } return nil } +func (r *DataScienceClusterReconciler) getRequestName() (string, error) { + instanceList := &dsc.DataScienceClusterList{} + err := r.Client.List(context.TODO(), instanceList) + if err != nil { + return "", err + } + + switch { + case len(instanceList.Items) == 1: + return instanceList.Items[0].Name, nil + case len(instanceList.Items) == 0: + return "default-dsc", nil + default: + return "", fmt.Errorf("multiple DataScienceCluster instances found") + } +} + // argoWorkflowCRDPredicates filters the delete events to trigger reconcile when Argo Workflow CRD is deleted. var argoWorkflowCRDPredicates = predicate.Funcs{ DeleteFunc: func(e event.DeleteEvent) bool { @@ -553,3 +556,34 @@ var argoWorkflowCRDPredicates = predicate.Funcs{ return true }, } + +func (r *DataScienceClusterReconciler) watchDefaultIngressSecret(a client.Object) []reconcile.Request { + requestName, err := r.getRequestName() + if err != nil { + return nil + } + // When ingress secret gets created/deleted, trigger reconcile function + ingressCtrl, err := cluster.FindAvailableIngressController(context.TODO(), r.Client) + if err != nil { + return nil + } + defaultIngressSecretName := cluster.GetDefaultIngressCertSecretName(ingressCtrl) + if a.GetName() == defaultIngressSecretName && a.GetNamespace() == "openshift-ingress" { + return []reconcile.Request{{ + NamespacedName: types.NamespacedName{Name: requestName}, + }} + } + return nil +} + +// defaultIngressCertSecretPredicates filters delete and create events to trigger reconcile when default ingress cert secret is expired +// or created. +var defaultIngressCertSecretPredicates = predicate.Funcs{ + CreateFunc: func(createEvent event.CreateEvent) bool { + return true + + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return true + }, +} diff --git a/controllers/datasciencecluster/kubebuilder_rbac.go b/controllers/datasciencecluster/kubebuilder_rbac.go index bbddb0186b5..d60b1b271c6 100644 --- a/controllers/datasciencecluster/kubebuilder_rbac.go +++ b/controllers/datasciencecluster/kubebuilder_rbac.go @@ -97,7 +97,8 @@ package datasciencecluster // +kubebuilder:rbac:groups="apiregistration.k8s.io",resources=apiservices,verbs=create;delete;list;watch;update;patch;get -// +kubebuilder:rbac:groups="operator.openshift.io",resources=consoles,verbs=list;watch;patch;delete;get +// +kubebuilder:rbac:groups="operator.openshift.io",resources=consoles,verbs=list;watch;patch;delete +// +kubebuilder:rbac:groups="operator.openshift.io",resources=ingresscontrollers,verbs=get;list;watch;patch;delete // +kubebuilder:rbac:groups="oauth.openshift.io",resources=oauthclients,verbs=create;delete;list;watch;update;patch;get diff --git a/docs/api-overview.md b/docs/api-overview.md index a7532f57773..618f23c1091 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -333,7 +333,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `secretName` _string_ | SecretName specifies the name of the Kubernetes Secret resource that contains a
TLS certificate secure HTTP communications for the KNative network. | | | -| `type` _[CertType](#certtype)_ | Type specifies if the TLS certificate should be generated automatically, or if the certificate
is provided by the user. Allowed values are:
* SelfSigned: A certificate is going to be generated using an own private key.
* Provided: Pre-existence of the TLS Secret (see SecretName) with a valid certificate is assumed. | SelfSigned | Enum: [SelfSigned Provided]
| +| `type` _[CertType](#certtype)_ | Type specifies if the TLS certificate should be generated automatically, or if the certificate
is provided by the user. Allowed values are:
* SelfSigned: A certificate is going to be generated using an own private key.
* Provided: Pre-existence of the TLS Secret (see SecretName) with a valid certificate is assumed.
* OpenshiftDefaultIngress: Default ingress certificate configured for OpenShift | OpenshiftDefaultIngress | Enum: [SelfSigned Provided OpenshiftDefaultIngress]
| #### Components diff --git a/main.go b/main.go index e355ffe9b6e..f44e9086ff1 100644 --- a/main.go +++ b/main.go @@ -26,6 +26,7 @@ import ( ocbuildv1 "github.com/openshift/api/build/v1" ocimgv1 "github.com/openshift/api/image/v1" ocv1 "github.com/openshift/api/oauth/v1" + operatorv1 "github.com/openshift/api/operator/v1" routev1 "github.com/openshift/api/route/v1" ocuserv1 "github.com/openshift/api/user/v1" ofapiv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" @@ -86,13 +87,14 @@ func init() { //nolint:gochecknoinits utilruntime.Must(ocuserv1.Install(scheme)) utilruntime.Must(ofapiv2.AddToScheme(scheme)) utilruntime.Must(kfdefv1.AddToScheme(scheme)) - utilruntime.Must(ocappsv1.AddToScheme(scheme)) - utilruntime.Must(ocbuildv1.AddToScheme(scheme)) - utilruntime.Must(ocimgv1.AddToScheme(scheme)) + utilruntime.Must(ocappsv1.Install(scheme)) + utilruntime.Must(ocbuildv1.Install(scheme)) + utilruntime.Must(ocimgv1.Install(scheme)) utilruntime.Must(apiextv1.AddToScheme(scheme)) utilruntime.Must(admv1.AddToScheme(scheme)) utilruntime.Must(apiregistrationv1.AddToScheme(scheme)) utilruntime.Must(monitoringv1.AddToScheme(scheme)) + utilruntime.Must(operatorv1.Install(scheme)) } func main() { diff --git a/pkg/cluster/cert.go b/pkg/cluster/cert.go new file mode 100644 index 00000000000..173245ff873 --- /dev/null +++ b/pkg/cluster/cert.go @@ -0,0 +1,225 @@ +package cluster + +import ( + "bytes" + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "net" + "strings" + "time" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/pkg/errors" + v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const DefaultCertificateSecretName = "knative-serving-cert" + +func CreateSelfSignedCertificate(ctx context.Context, c client.Client, secretName, domain, namespace string, metaOptions ...MetaOptions) error { + certSecret, err := GenerateSelfSignedCertificateAsSecret(secretName, domain, namespace) + if err != nil { + return fmt.Errorf("failed generating self-signed certificate: %w", err) + } + + if err := ApplyMetaOptions(certSecret, metaOptions...); err != nil { + return err + } + + if createErr := c.Create(ctx, certSecret); client.IgnoreAlreadyExists(createErr) != nil { + return fmt.Errorf("failed creating certificate secret: %w", createErr) + } + + return nil +} + +func GenerateSelfSignedCertificateAsSecret(name, addr, namespace string) (*v1.Secret, error) { + cert, key, err := generateCertificate(addr) + if err != nil { + return nil, errors.WithStack(err) + } + + return &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string][]byte{ + v1.TLSCertKey: cert, + v1.TLSPrivateKeyKey: key, + }, + }, nil +} + +func generateCertificate(addr string) ([]byte, []byte, error) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, nil, errors.WithStack(err) + } + + seededRand, cryptErr := rand.Int(rand.Reader, big.NewInt(time.Now().UnixNano())) + if cryptErr != nil { + return nil, nil, errors.WithStack(cryptErr) + } + + now := time.Now() + tmpl := x509.Certificate{ + SerialNumber: seededRand, + Subject: pkix.Name{ + CommonName: addr, + Organization: []string{"opendatahub-self-signed"}, + }, + NotBefore: now.UTC(), + NotAfter: now.Add(time.Second * 60 * 60 * 24 * 365).UTC(), + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + IsCA: true, + } + + if ip := net.ParseIP(addr); ip != nil { + tmpl.IPAddresses = append(tmpl.IPAddresses, ip) + } else { + if strings.HasPrefix(addr, "*.") { + tmpl.DNSNames = append(tmpl.DNSNames, addr[2:]) + } + tmpl.DNSNames = append(tmpl.DNSNames, addr) + } + + tmpl.DNSNames = append(tmpl.DNSNames, "localhost") + + certDERBytes, err := x509.CreateCertificate(rand.Reader, &tmpl, &tmpl, key.Public(), key) + if err != nil { + return nil, nil, errors.WithStack(err) + } + certificate, err := x509.ParseCertificate(certDERBytes) + if err != nil { + return nil, nil, errors.WithStack(err) + } + + certBuffer := bytes.Buffer{} + if err := pem.Encode(&certBuffer, &pem.Block{ + Type: "CERTIFICATE", + Bytes: certificate.Raw, + }); err != nil { + return nil, nil, errors.WithStack(err) + } + + keyBuffer := bytes.Buffer{} + if err := pem.Encode(&keyBuffer, &pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(key), + }); err != nil { + return nil, nil, errors.WithStack(err) + } + + return certBuffer.Bytes(), keyBuffer.Bytes(), nil +} + +// GetDefaultIngressCertificate copies ingress cert secrets from openshift-ingress ns to given namespace. +func GetDefaultIngressCertificate(ctx context.Context, c client.Client, knativeSecret, namespace string) error { + // Add IngressController to scheme + runtime.Must(operatorv1.Install(c.Scheme())) + defaultIngressCtrl, err := FindAvailableIngressController(ctx, c) + if err != nil { + return fmt.Errorf("failed to get ingress controller: %w", err) + } + + defaultIngressCertName := GetDefaultIngressCertSecretName(defaultIngressCtrl) + + defaultIngressSecret, err := GetSecret(ctx, c, "openshift-ingress", defaultIngressCertName) + if err != nil { + return err + } + + return copySecretToNamespace(ctx, c, defaultIngressSecret, knativeSecret, namespace) +} + +func FindAvailableIngressController(ctx context.Context, c client.Client) (*operatorv1.IngressController, error) { + defaultIngressCtrl := &operatorv1.IngressController{} + + err := c.Get(ctx, client.ObjectKey{Namespace: "openshift-ingress-operator", Name: "default"}, defaultIngressCtrl) + if err != nil { + return nil, fmt.Errorf("error getting ingresscontroller resource :%w", err) + } + return defaultIngressCtrl, nil +} + +func GetDefaultIngressCertSecretName(ingressCtrl *operatorv1.IngressController) string { + if ingressCtrl.Spec.DefaultCertificate != nil { + return ingressCtrl.Spec.DefaultCertificate.Name + } + return "router-certs-" + ingressCtrl.Name +} + +func GetSecret(ctx context.Context, c client.Client, namespace, name string) (*v1.Secret, error) { + secret := &v1.Secret{} + err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, secret) + if err != nil { + return nil, err + } + return secret, nil +} + +func copySecretToNamespace(ctx context.Context, c client.Client, secret *v1.Secret, newSecretName, namespace string) error { + // Get default name if newSecretName is empty + if newSecretName == "" { + newSecretName = DefaultCertificateSecretName + } + newSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: newSecretName, + Namespace: namespace, + }, + Data: secret.Data, + Type: secret.Type, + } + + existingSecret := &v1.Secret{} + err := c.Get(ctx, client.ObjectKey{Name: newSecretName, Namespace: namespace}, existingSecret) + if apierrors.IsNotFound(err) { + err = c.Create(ctx, newSecret) + if err != nil { + return err + } + } else if err == nil { + // Check if secret needs to be updated + if isSecretOutdated(existingSecret.Data, newSecret.Data) { + err = c.Update(ctx, newSecret) + if err != nil { + return err + } + } + return nil + } + + return err +} + +// isSecretOutdated compares two secret data of type map[string][]byte and returns true if they are not equal. +func isSecretOutdated(existingSecretData, newSecretData map[string][]byte) bool { + if len(existingSecretData) != len(newSecretData) { + return true + } + + for key, value1 := range existingSecretData { + value2, ok := newSecretData[key] + if !ok { + return true + } + if !bytes.Equal(value1, value2) { + return true + } + } + + return false +} diff --git a/pkg/feature/cert.go b/pkg/feature/cert.go deleted file mode 100644 index 8c829920221..00000000000 --- a/pkg/feature/cert.go +++ /dev/null @@ -1,128 +0,0 @@ -package feature - -import ( - "bytes" - "context" - "crypto/rand" - "crypto/rsa" - "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" - "fmt" - "math/big" - "net" - "strings" - "time" - - "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1" -) - -func (f *Feature) CreateSelfSignedCertificate(secretName string, certificateType infrav1.CertType, domain, namespace string) error { - if certificateType != infrav1.SelfSigned { - return nil - } - - meta := metav1.ObjectMeta{ - Name: secretName, - Namespace: namespace, - OwnerReferences: []metav1.OwnerReference{ - f.AsOwnerReference(), - }, - } - - certSecret, err := GenerateSelfSignedCertificateAsSecret(domain, meta) - if err != nil { - return fmt.Errorf("failed generating self-signed certificate: %w", err) - } - - if createErr := f.Client.Create(context.TODO(), certSecret); client.IgnoreAlreadyExists(createErr) != nil { - return fmt.Errorf("failed creating certificate secret: %w", createErr) - } - - return nil -} - -func GenerateSelfSignedCertificateAsSecret(addr string, objectMeta metav1.ObjectMeta) (*corev1.Secret, error) { - cert, key, err := generateCertificate(addr) - if err != nil { - return nil, errors.WithStack(err) - } - - return &corev1.Secret{ - ObjectMeta: objectMeta, - Data: map[string][]byte{ - corev1.TLSCertKey: cert, - corev1.TLSPrivateKeyKey: key, - }, - }, nil -} - -func generateCertificate(addr string) ([]byte, []byte, error) { - key, err := rsa.GenerateKey(rand.Reader, 2048) - if err != nil { - return nil, nil, errors.WithStack(err) - } - - seededRand, cryptErr := rand.Int(rand.Reader, big.NewInt(time.Now().UnixNano())) - if cryptErr != nil { - return nil, nil, errors.WithStack(cryptErr) - } - - now := time.Now() - tmpl := x509.Certificate{ - SerialNumber: seededRand, - Subject: pkix.Name{ - CommonName: addr, - Organization: []string{"opendatahub-self-signed"}, - }, - NotBefore: now.UTC(), - NotAfter: now.Add(time.Second * 60 * 60 * 24 * 365).UTC(), - KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, - BasicConstraintsValid: true, - IsCA: true, - } - - if ip := net.ParseIP(addr); ip != nil { - tmpl.IPAddresses = append(tmpl.IPAddresses, ip) - } else { - if strings.HasPrefix(addr, "*.") { - tmpl.DNSNames = append(tmpl.DNSNames, addr[2:]) - } - tmpl.DNSNames = append(tmpl.DNSNames, addr) - } - - tmpl.DNSNames = append(tmpl.DNSNames, "localhost") - - certDERBytes, err := x509.CreateCertificate(rand.Reader, &tmpl, &tmpl, key.Public(), key) - if err != nil { - return nil, nil, errors.WithStack(err) - } - certificate, err := x509.ParseCertificate(certDERBytes) - if err != nil { - return nil, nil, errors.WithStack(err) - } - - certBuffer := bytes.Buffer{} - if err := pem.Encode(&certBuffer, &pem.Block{ - Type: "CERTIFICATE", - Bytes: certificate.Raw, - }); err != nil { - return nil, nil, errors.WithStack(err) - } - - keyBuffer := bytes.Buffer{} - if err := pem.Encode(&keyBuffer, &pem.Block{ - Type: "RSA PRIVATE KEY", - Bytes: x509.MarshalPKCS1PrivateKey(key), - }); err != nil { - return nil, nil, errors.WithStack(err) - } - - return certBuffer.Bytes(), keyBuffer.Bytes(), nil -} diff --git a/pkg/feature/serverless/loaders.go b/pkg/feature/serverless/loaders.go index 58867b29f99..79c8cc00d15 100644 --- a/pkg/feature/serverless/loaders.go +++ b/pkg/feature/serverless/loaders.go @@ -8,12 +8,10 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature" ) -const DefaultCertificateSecretName = "knative-serving-cert" - func ServingDefaultValues(f *feature.Feature) error { certificateSecretName := strings.TrimSpace(f.Spec.Serving.IngressGateway.Certificate.SecretName) if len(certificateSecretName) == 0 { - certificateSecretName = DefaultCertificateSecretName + certificateSecretName = cluster.DefaultCertificateSecretName } f.Spec.KnativeCertificateSecret = certificateSecretName diff --git a/pkg/feature/serverless/resources.go b/pkg/feature/serverless/resources.go index e3360ddef0c..12700538743 100644 --- a/pkg/feature/serverless/resources.go +++ b/pkg/feature/serverless/resources.go @@ -1,9 +1,24 @@ package serverless import ( + "context" + + infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature" ) func ServingCertificateResource(f *feature.Feature) error { - return f.CreateSelfSignedCertificate(f.Spec.KnativeCertificateSecret, f.Spec.Serving.IngressGateway.Certificate.Type, f.Spec.KnativeIngressDomain, f.Spec.ControlPlane.Namespace) + switch certType := f.Spec.Serving.IngressGateway.Certificate.Type; certType { + case infrav1.SelfSigned: + return cluster.CreateSelfSignedCertificate(context.TODO(), f.Client, + f.Spec.KnativeCertificateSecret, + f.Spec.KnativeIngressDomain, + f.Spec.ControlPlane.Namespace, + feature.OwnedBy(f)) + case infrav1.Provided: + return nil + default: + return cluster.GetDefaultIngressCertificate(context.TODO(), f.Client, f.Spec.KnativeCertificateSecret, f.Spec.ControlPlane.Namespace) + } } diff --git a/tests/e2e/controller_setup_test.go b/tests/e2e/controller_setup_test.go index 68e5e0b1547..9cfc9056eea 100644 --- a/tests/e2e/controller_setup_test.go +++ b/tests/e2e/controller_setup_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + operatorv1 "github.com/openshift/api/operator/v1" routev1 "github.com/openshift/api/route/v1" ofapi "github.com/operator-framework/api/pkg/operators/v1alpha1" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" @@ -106,6 +107,7 @@ func TestOdhOperator(t *testing.T) { utilruntime.Must(featurev1.AddToScheme(scheme)) utilruntime.Must(monitoringv1.AddToScheme(scheme)) utilruntime.Must(ofapi.AddToScheme(scheme)) + utilruntime.Must(operatorv1.AddToScheme(scheme)) // individual test suites after the operator is running if !t.Run("validate operator pod is running", testODHOperatorValidation) { diff --git a/tests/e2e/dsc_creation_test.go b/tests/e2e/dsc_creation_test.go index 47bbc6b7ccf..461a5f25d56 100644 --- a/tests/e2e/dsc_creation_test.go +++ b/tests/e2e/dsc_creation_test.go @@ -22,6 +22,7 @@ import ( dsci "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1" "github.com/opendatahub-io/opendatahub-operator/v2/components" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" ) const ( @@ -68,6 +69,10 @@ func creationTestSuite(t *testing.T) { err = testCtx.testOwnerrefrences() require.NoError(t, err, "error getting all DataScienceCluster's Ownerrefrences") }) + t.Run("Validate default certs available", func(t *testing.T) { + err = testCtx.testDefaultCertsAvailable() + require.NoError(t, err, "error getting default cert secrets for Kserve") + }) t.Run("Validate Controller reconcile", func(t *testing.T) { // only test Dashboard component for now err = testCtx.testUpdateComponentReconcile() @@ -333,11 +338,11 @@ func (tc *testContext) validateDSCI() error { func (tc *testContext) validateDSC() error { expServingSpec := infrav1.ServingSpec{ - ManagementState: operatorv1.Unmanaged, + ManagementState: operatorv1.Managed, Name: "knative-serving", IngressGateway: infrav1.IngressGatewaySpec{ Certificate: infrav1.CertificateSpec{ - Type: infrav1.SelfSigned, + Type: infrav1.OpenshiftDefaultIngress, }, }, } @@ -371,6 +376,40 @@ func (tc *testContext) testOwnerrefrences() error { return nil } +func (tc *testContext) testDefaultCertsAvailable() error { + // Get expected cert secrets + defaultIngressCtrl, err := cluster.FindAvailableIngressController(tc.ctx, tc.customClient) + if err != nil { + return fmt.Errorf("failed to get ingress controller: %w", err) + } + + defaultIngressCertName := cluster.GetDefaultIngressCertSecretName(defaultIngressCtrl) + + defaultIngressSecret, err := cluster.GetSecret(tc.ctx, tc.customClient, "openshift-ingress", defaultIngressCertName) + if err != nil { + return err + } + + // Verify secret from Control Plane namespace matches the default cert secret + defaultSecretName := tc.testDsc.Spec.Components.Kserve.Serving.IngressGateway.Certificate.SecretName + if defaultSecretName == "" { + defaultSecretName = cluster.DefaultCertificateSecretName + } + ctrlPlaneSecret, err := cluster.GetSecret(tc.ctx, tc.customClient, tc.testDSCI.Spec.ServiceMesh.ControlPlane.Namespace, + defaultSecretName) + if err != nil { + return err + } + if string(defaultIngressSecret.Data["tls.crt"]) != string(ctrlPlaneSecret.Data["tls.crt"]) { + return fmt.Errorf("default cert secret not expected. Epected %v, Got %v", defaultIngressSecret.Data["tls.crt"], ctrlPlaneSecret.Data["tls.crt"]) + } + + if string(defaultIngressSecret.Data["tls.key"]) != string(ctrlPlaneSecret.Data["tls.key"]) { + return fmt.Errorf("default cert secret not expected. Epected %v, Got %v", defaultIngressSecret.Data["tls.crt"], ctrlPlaneSecret.Data["tls.crt"]) + } + return nil +} + func (tc *testContext) testUpdateComponentReconcile() error { // Test Updating Dashboard Replicas diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index 6282b981961..108bcad1bd8 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -123,7 +123,7 @@ func setupDSCInstance(name string) *dsc.DataScienceCluster { ManagementState: operatorv1.Managed, }, Serving: infrav1.ServingSpec{ - ManagementState: operatorv1.Unmanaged, + ManagementState: operatorv1.Managed, }, }, CodeFlare: codeflare.CodeFlare{ diff --git a/tests/integration/features/serverless_feature_test.go b/tests/integration/features/serverless_feature_test.go index 32690e2d0b6..2b5e6455d50 100644 --- a/tests/integration/features/serverless_feature_test.go +++ b/tests/integration/features/serverless_feature_test.go @@ -15,6 +15,7 @@ import ( dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1" "github.com/opendatahub-io/opendatahub-operator/v2/components/kserve" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature/serverless" "github.com/opendatahub-io/opendatahub-operator/v2/tests/envtestutil" @@ -183,7 +184,7 @@ var _ = Describe("Serverless feature", func() { It("should set default value when value is empty in the DSCI", func() { // Default value is blank -> testFeature.Spec.Serving.IngressGateway.Certificate.SecretName = "" Expect(serverless.ServingDefaultValues(testFeature)).To(Succeed()) - Expect(testFeature.Spec.KnativeCertificateSecret).To(Equal(serverless.DefaultCertificateSecretName)) + Expect(testFeature.Spec.KnativeCertificateSecret).To(Equal(cluster.DefaultCertificateSecretName)) }) It("should use user value when set in the DSCI", func() { @@ -261,7 +262,7 @@ var _ = Describe("Serverless feature", func() { // then Eventually(func() error { - secret, err := envTestClientset.CoreV1().Secrets(namespace.Name).Get(context.TODO(), serverless.DefaultCertificateSecretName, metav1.GetOptions{}) + secret, err := envTestClientset.CoreV1().Secrets(namespace.Name).Get(context.TODO(), cluster.DefaultCertificateSecretName, metav1.GetOptions{}) if err != nil { return err }