Add support for default Ingress Cert (opendatahub-io#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 <[email protected]> * Change default value to OpenshiftDefaultIngress * Update default secret * Add e2e tests * Update tests for Managed Serving * Update serverless tests * Sync manifests
zdtsw-forking · Jun 19, 2024 · d8e33c1 · d8e33c1
1 parent 18636eb
commit d8e33c1
Show file tree

Hide file tree

Showing 19 changed files with 393 additions and 177 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -333,7 +333,7 @@ _Appears in:_
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
 | `secretName` _string_ | SecretName specifies the name of the Kubernetes Secret resource that contains a<br />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<br />is provided by the user. Allowed values are:<br />* SelfSigned: A certificate is going to be generated using an own private key.<br />* Provided: Pre-existence of the TLS Secret (see SecretName) with a valid certificate is assumed. | SelfSigned | Enum: [SelfSigned Provided] <br /> |
+| `type` _[CertType](#certtype)_ | Type specifies if the TLS certificate should be generated automatically, or if the certificate<br />is provided by the user. Allowed values are:<br />* SelfSigned: A certificate is going to be generated using an own private key.<br />* Provided: Pre-existence of the TLS Secret (see SecretName) with a valid certificate is assumed.<br />* OpenshiftDefaultIngress: Default ingress certificate configured for OpenShift | OpenshiftDefaultIngress | Enum: [SelfSigned Provided OpenshiftDefaultIngress] <br /> |
 
 
 #### Components

diff --git 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() {