udpate: code review

- fix bug for the wrong type of authentication check
- add unit test on the OIDC part
- fix typo

Signed-off-by: Wen Zhou <[email protected]>

Author: zdtsw

internal/controller/dscinitialization/auth.go

@@ -75,7 +75,7 @@ func BuildDefaultAuth(ctx context.Context, cli client.Client, platform common.Pl
 	isOAuth, err := cluster.IsIntegratedOAuth(ctx, cli)
 
 	if err == nil && !isOAuth {
-		// OIDC mode: set an non-exist group that cluster admin shoulud replace with actual OIDC group.
+		// OIDC mode: set an non-exist group that cluster admin should replace with actual OIDC group.
 		adminGroup = "REPLACE-WITH-OIDC-ADMIN-GROUP"
 	} else {
 		// OAuth mode: Use platform-specific admin group

internal/controller/dscinitialization/auth_test.go

@@ -90,6 +90,19 @@ func TestBuildDefaultAuth(t *testing.T) {
 			g.Expect(auth.Spec.AllowedGroups[0]).Should(Equal("system:authenticated"))
 		})
 	}
+	t.Run("OIDC mode uses placeholder group", func(t *testing.T) {
+		g := NewWithT(t)
+		ctx := t.Context()
+		cli, err := fakeclient.New(fakeclient.WithClusterAuthType(cluster.AuthModeOIDC))
+		g.Expect(err).ShouldNot(HaveOccurred())
+		obj := dscinitialization.BuildDefaultAuth(ctx, cli, cluster.OpenDataHub)
+		auth, ok := obj.(*serviceApi.Auth)
+		g.Expect(ok).To(BeTrue())
+		g.Expect(auth.Spec.AdminGroups).To(HaveLen(1))
+		g.Expect(auth.Spec.AdminGroups[0]).To(Equal("REPLACE-WITH-OIDC-ADMIN-GROUP"))
+		// AllowedGroups should remain unchanged
+		g.Expect(auth.Spec.AllowedGroups).To(Equal([]string{"system:authenticated"}))
+	})
 }
 
 func TestCreateAuth(t *testing.T) {

internal/controller/services/auth/auth_controller_actions.go

@@ -184,7 +184,7 @@ func createDefaultGroup(ctx context.Context, rr *odhtypes.ReconciliationRequest)
 		return err
 	}
 	if !ok {
-		logf.Log.Info("default auth method is not enabled")
+		logf.Log.Info("Integrated OAuth not detected; skipping default group creation")
 		return nil
 	}
 

pkg/cluster/cluster_config.go

@@ -379,8 +379,11 @@ func setManagedMonitoringNamespace(ctx context.Context, cli client.Client) error
 func GetClusterAuthenticationMode(ctx context.Context, cli client.Client) (AuthenticationMode, error) {
 	auth := &configv1.Authentication{}
 	if err := cli.Get(ctx, client.ObjectKey{Name: ClusterAuthenticationObj}, auth); err != nil {
-		if errors.Is(err, &meta.NoKindMatchError{}) { // when CRD is missing, convert error type
-			return "", k8serr.NewNotFound(schema.GroupResource{Group: gvk.Auth.Group}, ClusterAuthenticationObj)
+		if meta.IsNoMatchError(err) { // CRD missing
+			return "", k8serr.NewNotFound(schema.GroupResource{
+				Group:    configv1.GroupName,
+				Resource: "authentications",
+			}, ClusterAuthenticationObj)
 		}
 		return "", fmt.Errorf("failed to get cluster authentication config: %w", err)
 	}

pkg/utils/test/fakeclient/fakeclient.go

@@ -3,12 +3,15 @@ package fakeclient
 import (
 	"fmt"
 
+	configv1 "github.com/openshift/api/config/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	clientFake "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/scheme"
@@ -39,6 +42,23 @@ func WithScheme(value *runtime.Scheme) ClientOpts {
 	}
 }
 
+// WithClusterAuthType creates a fake OpenShift Authentication object with the specified type.
+// This is useful for testing authentication mode detection logic.
+// authType should be one of cluster.AuthModeOIDC, cluster.AuthModeIntegratedOAuth, or cluster.AuthModeNone constants.
+func WithClusterAuthType(authType cluster.AuthenticationMode) ClientOpts {
+	return func(o *clientOptions) {
+		auth := &configv1.Authentication{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: cluster.ClusterAuthenticationObj,
+			},
+			Spec: configv1.AuthenticationSpec{
+				Type: configv1.AuthenticationType(authType),
+			},
+		}
+		o.objects = append(o.objects, auth)
+	}
+}
+
 func New(opts ...ClientOpts) (client.Client, error) {
 	co := clientOptions{}
 	for _, o := range opts {