Skip to content

Commit

Permalink
feat: Operator disable create usergroup if detect users enabled exter…
Browse files Browse the repository at this point in the history
…nal auth

- user internal Authentication CR Type indicate if Operator should create or not
- only grant "get, watch , list" as least permission
- remove duplicated rbac for "ingress"
- add object into cache

Signed-off-by: Wen Zhou <[email protected]>
  • Loading branch information
zdtsw committed Oct 11, 2024
1 parent a918fed commit e4ffeb6
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 26 deletions.
21 changes: 1 addition & 20 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,26 +66,7 @@ Additionally installing `Authorino operator` & `Service Mesh operator` enhances
sourceNamespace: openshift-marketplace
EOF
```
If user would prefer skipping group "odh-admin" to be created by DSCI CR automatically, explicitly set env variable ODH_USE_EXTERNAL_AUTH to "true". example:

```console
cat <<EOF | oc create -f -
apiVersion: operators.coreos.com/v1alpha1
kind: Subscription
metadata:
name: opendatahub-operator
namespace: openshift-operators
spec:
channel: fast
name: opendatahub-operator
source: community-operators
sourceNamespace: openshift-marketplace
config:
env:
- name: "ODH_USE_EXTERNAL_AUTH"
value: "true"
EOF

2. Create [DSCInitialization](#example-dscinitialization) CR manually.
You can also use operator to create default DSCI CR by removing env variable DISABLE_DSC_CONFIG from CSV or changing the value to "false", followed by restarting the operator pod.

Expand Down
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
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
13 changes: 9 additions & 4 deletions controllers/dscinitialization/dscinitialization_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package dscinitialization

import (
"context"
"os"
"path/filepath"
"reflect"

Expand Down Expand Up @@ -74,6 +73,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
Expand All @@ -84,6 +84,11 @@ func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Re
// Set platform
platform := currentOperatorRelease.Name

createUsergroup, err := cluster.IsDefaultAuthMethod(ctx, r.Client)
if err != nil {
return ctrl.Result{}, err
}

instances := &dsciv1.DSCInitializationList{}
if err := r.Client.List(ctx, instances); err != nil {
log.Error(err, "Failed to retrieve DSCInitialization resource.", "DSCInitialization Request.Name", req.Name)
Expand Down Expand Up @@ -155,7 +160,7 @@ func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Re

// Check namespace is not exist, then create
namespace := instance.Spec.ApplicationsNamespace
err := r.createOdhNamespace(ctx, instance, namespace, platform)
err = r.createOdhNamespace(ctx, instance, namespace, platform)
if err != nil {
// no need to log error as it was already logged in createOdhNamespace
return reconcile.Result{}, err
Expand Down Expand Up @@ -202,7 +207,7 @@ func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Re
switch platform {
case cluster.SelfManagedRhods:
// Check if user opted for disabling creating user groups
if os.Getenv("ODH_USE_EXTERNAL_AUTH") == "true" {
if !createUsergroup {
log.Info("DSCI disabled usergroup creation")
} else {
err := r.createUserGroup(ctx, instance, "rhods-admins")
Expand Down Expand Up @@ -239,7 +244,7 @@ func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Re
}
default:
// Check if user opted for disabling creating user groups
if os.Getenv("ODH_USE_EXTERNAL_AUTH") == "true" {
if !createUsergroup {
log.Info("DSCI disabled usergroup creation")
} else {
err := r.createUserGroup(ctx, instance, "odh-admins")
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 @@ -198,6 +199,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"}.AsSelector(),
},
// for prometheus and black-box deployment and ones we owns
&appsv1.Deployment{}: {Namespaces: deploymentCache},
},
Expand Down
15 changes: 13 additions & 2 deletions pkg/cluster/cluster_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ 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"
Expand All @@ -21,8 +22,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
Expand Down Expand Up @@ -215,3 +214,15 @@ 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 should 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{Namespace: "", Name: "cluster"}, authenticationobj); err != nil {
// here: true or false not matter once error is returned
return true, fmt.Errorf("failed to get Authentication CR cluster: %w", err)
}

return (authenticationobj.Spec.Type == "IntegratedOAuth" || authenticationobj.Spec.Type == ""), nil
}

0 comments on commit e4ffeb6

Please sign in to comment.