Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Operator disable create usergroup if detect user enabled external auth #1297

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ spec:
- apiGroups:
- config.openshift.io
resources:
- authentications
- clusterversions
verbs:
- get
Expand Down
175 changes: 175 additions & 0 deletions config/crd/external/config.openshift.io_authentications.yaml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ rules:
- apiGroups:
- config.openshift.io
resources:
- authentications
- clusterversions
verbs:
- get
Expand Down
29 changes: 23 additions & 6 deletions controllers/dscinitialization/dscinitialization_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
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"
Expand Down Expand Up @@ -73,6 +74,7 @@
// +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
Expand Down Expand Up @@ -212,11 +214,21 @@

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

Check warning on line 219 in controllers/dscinitialization/dscinitialization_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/dscinitialization/dscinitialization_controller.go#L219

Added line #L219 was not covered by tests
}

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

Check warning on line 230 in controllers/dscinitialization/dscinitialization_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/dscinitialization/dscinitialization_controller.go#L225-L230

Added lines #L225 - L230 were not covered by tests
}
}
if instance.Spec.Monitoring.ManagementState == operatorv1.Managed {
log.Info("Monitoring enabled, won't apply changes", "cluster", "Self-Managed RHODS Mode")
Expand Down Expand Up @@ -246,9 +258,14 @@
}
}
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

Check warning on line 267 in controllers/dscinitialization/dscinitialization_controller.go

View check run for this annotation

Codecov / codecov/patch

controllers/dscinitialization/dscinitialization_controller.go#L265-L267

Added lines #L265 - L267 were not covered by tests
}
}
if instance.Spec.Monitoring.ManagementState == operatorv1.Managed {
log.Info("Monitoring enabled, won't apply changes", "cluster", "ODH Mode")
Expand Down
14 changes: 12 additions & 2 deletions controllers/dscinitialization/dscinitialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions controllers/dscinitialization/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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})
Expand Down
5 changes: 5 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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},
},
Expand Down
21 changes: 19 additions & 2 deletions pkg/cluster/cluster_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@

"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"
Expand All @@ -21,8 +23,6 @@
"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
Expand Down Expand Up @@ -226,3 +226,20 @@
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)

Check warning on line 236 in pkg/cluster/cluster_config.go

View check run for this annotation

Codecov / codecov/patch

pkg/cluster/cluster_config.go#L232-L236

Added lines #L232 - L236 were not covered by tests
}
return false, err

Check warning on line 238 in pkg/cluster/cluster_config.go

View check run for this annotation

Codecov / codecov/patch

pkg/cluster/cluster_config.go#L238

Added line #L238 was not covered by tests
}

// 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

Check warning on line 244 in pkg/cluster/cluster_config.go

View check run for this annotation

Codecov / codecov/patch

pkg/cluster/cluster_config.go#L244

Added line #L244 was not covered by tests
}
3 changes: 3 additions & 0 deletions pkg/cluster/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ const (

// DefaultNotebooksNamespace defines default namespace for notebooks.
DefaultNotebooksNamespace = "rhods-notebooks"

// Default cluster-scope Authentication CR name.
ClusterAuthenticationObj = "cluster"
)