diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index 1519a9ef8ca..7ec954e1a03 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -357,6 +357,7 @@ spec: - apiGroups: - config.openshift.io resources: + - authentications - clusterversions verbs: - get diff --git a/config/crd/external/config.openshift.io_authentications.yaml b/config/crd/external/config.openshift.io_authentications.yaml new file mode 100644 index 00000000000..86ad306c7fd --- /dev/null +++ b/config/crd/external/config.openshift.io_authentications.yaml @@ -0,0 +1,175 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.16.1 + name: authentications.config.openshift.io +spec: + group: config.openshift.io + names: + kind: Authentication + listKind: AuthenticationList + plural: authentications + singular: authentication + scope: Cluster + versions: + - name: v1 + schema: + openAPIV3Schema: + description: |- + Authentication specifies cluster-wide settings for authentication (like OAuth and + webhook token authenticators). The canonical name of an instance is `cluster`. + + Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer). + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: spec holds user settable values for configuration + properties: + oauthMetadata: + description: |- + oauthMetadata contains the discovery endpoint data for OAuth 2.0 + Authorization Server Metadata for an external OAuth server. + This discovery document can be viewed from its served location: + oc get --raw '/.well-known/oauth-authorization-server' + For further details, see the IETF Draft: + https://tools.ietf.org/html/draft-ietf-oauth-discovery-04#section-2 + If oauthMetadata.name is non-empty, this value has precedence + over any metadata reference stored in status. + The key "oauthMetadata" is used to locate the data. + If specified and the config map or expected key is not found, no metadata is served. + If the specified metadata is not valid, no metadata is served. + The namespace for this config map is openshift-config. + properties: + name: + description: name is the metadata.name of the referenced config + map + type: string + required: + - name + type: object + serviceAccountIssuer: + description: |- + serviceAccountIssuer is the identifier of the bound service account token + issuer. + The default is https://kubernetes.default.svc + WARNING: Updating this field will not result in immediate invalidation of all bound tokens with the + previous issuer value. Instead, the tokens issued by previous service account issuer will continue to + be trusted for a time period chosen by the platform (currently set to 24h). + This time period is subject to change over time. + This allows internal components to transition to use new service account issuer without service distruption. + type: string + type: + description: |- + type identifies the cluster managed, user facing authentication mode in use. + Specifically, it manages the component that responds to login attempts. + The default is IntegratedOAuth. + type: string + webhookTokenAuthenticator: + description: |- + webhookTokenAuthenticator configures a remote token reviewer. + These remote authentication webhooks can be used to verify bearer tokens + via the tokenreviews.authentication.k8s.io REST API. This is required to + honor bearer tokens that are provisioned by an external authentication service. + properties: + kubeConfig: + description: |- + kubeConfig references a secret that contains kube config file data which + describes how to access the remote webhook service. + The namespace for the referenced secret is openshift-config. + + For further details, see: + + https://kubernetes.io/docs/reference/access-authn-authz/authentication/#webhook-token-authentication + + The key "kubeConfig" is used to locate the data. + If the secret or expected key is not found, the webhook is not honored. + If the specified kube config data is not valid, the webhook is not honored. + properties: + name: + description: name is the metadata.name of the referenced secret + type: string + required: + - name + type: object + required: + - kubeConfig + type: object + webhookTokenAuthenticators: + description: webhookTokenAuthenticators is DEPRECATED, setting it + has no effect. + items: + description: |- + deprecatedWebhookTokenAuthenticator holds the necessary configuration options for a remote token authenticator. + It's the same as WebhookTokenAuthenticator but it's missing the 'required' validation on KubeConfig field. + properties: + kubeConfig: + description: |- + kubeConfig contains kube config file data which describes how to access the remote webhook service. + For further details, see: + https://kubernetes.io/docs/reference/access-authn-authz/authentication/#webhook-token-authentication + The key "kubeConfig" is used to locate the data. + If the secret or expected key is not found, the webhook is not honored. + If the specified kube config data is not valid, the webhook is not honored. + The namespace for this secret is determined by the point of use. + properties: + name: + description: name is the metadata.name of the referenced + secret + type: string + required: + - name + type: object + type: object + type: array + type: object + status: + description: status holds observed values from the cluster. They may not + be overridden. + properties: + integratedOAuthMetadata: + description: |- + integratedOAuthMetadata contains the discovery endpoint data for OAuth 2.0 + Authorization Server Metadata for the in-cluster integrated OAuth server. + This discovery document can be viewed from its served location: + oc get --raw '/.well-known/oauth-authorization-server' + For further details, see the IETF Draft: + https://tools.ietf.org/html/draft-ietf-oauth-discovery-04#section-2 + This contains the observed value based on cluster state. + An explicitly set value in spec.oauthMetadata has precedence over this field. + This field has no meaning if authentication spec.type is not set to IntegratedOAuth. + The key "oauthMetadata" is used to locate the data. + If the config map or expected key is not found, no metadata is served. + If the specified metadata is not valid, no metadata is served. + The namespace for this config map is openshift-config-managed. + properties: + name: + description: name is the metadata.name of the referenced config + map + type: string + required: + - name + type: object + type: object + required: + - spec + type: object + served: true + storage: true diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index d81b97e1555..01ab8982756 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -171,6 +171,7 @@ rules: - apiGroups: - config.openshift.io resources: + - authentications - clusterversions verbs: - get diff --git a/controllers/dscinitialization/dscinitialization_controller.go b/controllers/dscinitialization/dscinitialization_controller.go index 37991131cb5..845418ac843 100644 --- a/controllers/dscinitialization/dscinitialization_controller.go +++ b/controllers/dscinitialization/dscinitialization_controller.go @@ -29,6 +29,7 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" @@ -73,6 +74,7 @@ type DSCInitializationReconciler struct { // +kubebuilder:rbac:groups="dscinitialization.opendatahub.io",resources=dscinitializations,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups="features.opendatahub.io",resources=featuretrackers,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups="features.opendatahub.io",resources=featuretrackers/status,verbs=get;update;patch;delete +// +kubebuilder:rbac:groups="config.openshift.io",resources=authentications,verbs=get;watch;list // Reconcile contains controller logic specific to DSCInitialization instance updates. func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:funlen,gocyclo,maintidx @@ -212,11 +214,21 @@ func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, nil default: + createUsergroup, err := cluster.IsDefaultAuthMethod(ctx, r.Client) + if err != nil && !k8serr.IsNotFound(err) { // only keep reconcile if real error but not missing CRD or missing CR + return ctrl.Result{}, err + } + switch platform { case cluster.SelfManagedRhods: - err := r.createUserGroup(ctx, instance, "rhods-admins") - if err != nil { - return reconcile.Result{}, err + // Check if user opted for disabling creating user groups + if !createUsergroup { + log.Info("DSCI disabled usergroup creation") + } else { + err := r.createUserGroup(ctx, instance, "rhods-admins") + if err != nil { + return reconcile.Result{}, err + } } if instance.Spec.Monitoring.ManagementState == operatorv1.Managed { log.Info("Monitoring enabled, won't apply changes", "cluster", "Self-Managed RHODS Mode") @@ -246,9 +258,14 @@ func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Re } } default: - err := r.createUserGroup(ctx, instance, "odh-admins") - if err != nil { - return reconcile.Result{}, err + // Check if user opted for disabling creating user groups + if !createUsergroup { + log.Info("DSCI disabled usergroup creation") + } else { + err := r.createUserGroup(ctx, instance, "odh-admins") + if err != nil { + return reconcile.Result{}, err + } } if instance.Spec.Monitoring.ManagementState == operatorv1.Managed { log.Info("Monitoring enabled, won't apply changes", "cluster", "ODH Mode") diff --git a/controllers/dscinitialization/dscinitialization_test.go b/controllers/dscinitialization/dscinitialization_test.go index d380375ef63..d9afb51deb4 100644 --- a/controllers/dscinitialization/dscinitialization_test.go +++ b/controllers/dscinitialization/dscinitialization_test.go @@ -4,6 +4,7 @@ import ( "context" operatorv1 "github.com/openshift/api/operator/v1" + userv1 "github.com/openshift/api/user/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -21,6 +22,7 @@ const ( workingNamespace = "test-operator-ns" applicationName = "default-dsci" applicationNamespace = "test-application-ns" + usergroupName = "odh-admins" configmapName = "odh-common-config" monitoringNamespace = "test-monitoring-ns" readyPhase = "Ready" @@ -109,6 +111,14 @@ var _ = Describe("DataScienceCluster initialization", func() { Expect(foundConfigMap.Data).To(Equal(expectedConfigmapData)) }) + It("Should not create user group when we do not have authentications CR in the cluster", func(ctx context.Context) { + userGroup := &userv1.Group{} + Eventually(objectExists(usergroupName, "", userGroup)). + WithContext(ctx). + WithTimeout(timeout). + WithPolling(interval). + Should(BeFalse()) + }) }) Context("Monitoring Resource", func() { @@ -341,9 +351,9 @@ func namespaceExists(ns string, obj client.Object) func(ctx context.Context) boo } } -func objectExists(ns string, name string, obj client.Object) func(ctx context.Context) bool { //nolint:unparam +func objectExists(name string, namespace string, obj client.Object) func(ctx context.Context) bool { return func(ctx context.Context) bool { - err := k8sClient.Get(ctx, client.ObjectKey{Name: ns, Namespace: name}, obj) + err := k8sClient.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, obj) return err == nil } diff --git a/controllers/dscinitialization/suite_test.go b/controllers/dscinitialization/suite_test.go index f3ef428f878..985618bacf8 100644 --- a/controllers/dscinitialization/suite_test.go +++ b/controllers/dscinitialization/suite_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + configv1 "github.com/openshift/api/config/v1" routev1 "github.com/openshift/api/route/v1" userv1 "github.com/openshift/api/user/v1" ofapi "github.com/operator-framework/api/pkg/operators/v1alpha1" @@ -116,6 +117,7 @@ var _ = BeforeSuite(func() { utilruntime.Must(routev1.Install(testScheme)) utilruntime.Must(userv1.Install(testScheme)) utilruntime.Must(monitoringv1.AddToScheme(testScheme)) + utilruntime.Must(configv1.Install(testScheme)) // +kubebuilder:scaffold:scheme k8sClient, err = client.New(cfg, client.Options{Scheme: testScheme}) diff --git a/main.go b/main.go index c7d38cfb1c3..6eb6cffe420 100644 --- a/main.go +++ b/main.go @@ -25,6 +25,7 @@ import ( addonv1alpha1 "github.com/openshift/addon-operator/apis/addons/v1alpha1" ocappsv1 "github.com/openshift/api/apps/v1" //nolint:importas //reason: conflicts with appsv1 "k8s.io/api/apps/v1" buildv1 "github.com/openshift/api/build/v1" + configv1 "github.com/openshift/api/config/v1" imagev1 "github.com/openshift/api/image/v1" oauthv1 "github.com/openshift/api/oauth/v1" operatorv1 "github.com/openshift/api/operator/v1" @@ -201,6 +202,10 @@ func main() { //nolint:funlen,maintidx &operatorv1.IngressController{}: { Field: fields.Set{"metadata.name": "default"}.AsSelector(), }, + // For authentication CR "cluster" + &configv1.Authentication{}: { + Field: fields.Set{"metadata.name": cluster.ClusterAuthenticationObj}.AsSelector(), + }, // for prometheus and black-box deployment and ones we owns &appsv1.Deployment{}: {Namespaces: deploymentCache}, }, diff --git a/pkg/cluster/cluster_config.go b/pkg/cluster/cluster_config.go index 442878baf2f..e0e31fa199c 100644 --- a/pkg/cluster/cluster_config.go +++ b/pkg/cluster/cluster_config.go @@ -9,10 +9,12 @@ import ( "github.com/blang/semver/v4" "github.com/go-logr/logr" + configv1 "github.com/openshift/api/config/v1" "github.com/operator-framework/api/pkg/lib/version" ofapiv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" @@ -21,8 +23,6 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" ) -// +kubebuilder:rbac:groups="config.openshift.io",resources=ingresses,verbs=get - type Platform string // Release includes information on operator version and platform @@ -226,3 +226,20 @@ func getRelease(ctx context.Context, cli client.Client) (Release, error) { initRelease.Version = csv.Spec.Version return initRelease, nil } + +// IsDefaultAuthMethod returns true if the default authentication method is IntegratedOAuth or empty. +// This will give indication that Operator should create userGroups or not in the cluster. +func IsDefaultAuthMethod(ctx context.Context, cli client.Client) (bool, error) { + authenticationobj := &configv1.Authentication{} + if err := cli.Get(ctx, client.ObjectKey{Name: ClusterAuthenticationObj, Namespace: ""}, authenticationobj); err != nil { + if errors.Is(err, &meta.NoKindMatchError{}) { // when CRD is missing, conver error type + return false, k8serr.NewNotFound(configv1.Resource("authentications"), ClusterAuthenticationObj) + } + return false, err + } + + // for now, HPC support "" "None" "IntegratedOAuth"(default) "OIDC" + // other offering support "" "None" "IntegratedOAuth"(default) + // we only create userGroups for "IntegratedOAuth" or "" and leave other or new supported type value in the future + return authenticationobj.Spec.Type == configv1.AuthenticationTypeIntegratedOAuth || authenticationobj.Spec.Type == "", nil +} diff --git a/pkg/cluster/const.go b/pkg/cluster/const.go index 5e218430db7..de5a27de29d 100644 --- a/pkg/cluster/const.go +++ b/pkg/cluster/const.go @@ -12,4 +12,7 @@ const ( // DefaultNotebooksNamespace defines default namespace for notebooks. DefaultNotebooksNamespace = "rhods-notebooks" + + // Default cluster-scope Authentication CR name. + ClusterAuthenticationObj = "cluster" )