diff --git a/controllers/toolchainconfig/configuration.go b/controllers/toolchainconfig/configuration.go index dbb876a08..d7cb6fd57 100644 --- a/controllers/toolchainconfig/configuration.go +++ b/controllers/toolchainconfig/configuration.go @@ -135,10 +135,12 @@ func (a AutoApprovalConfig) IsEnabled() bool { return commonconfig.GetBool(a.approval.Enabled, false) } +// Domains converts comma separated domains string in toolchain config manifest to a slice of domains +// Having no domains defined in manifest or empty domains string means all domains. func (a AutoApprovalConfig) Domains() []string { domains := commonconfig.GetString(a.approval.Domains, "") v := strings.FieldsFunc(domains, func(c rune) bool { - return c == ',' + return c == ',' || c == ' ' }) return v } diff --git a/controllers/toolchainconfig/configuration_test.go b/controllers/toolchainconfig/configuration_test.go index ddd093098..18c45b233 100644 --- a/controllers/toolchainconfig/configuration_test.go +++ b/controllers/toolchainconfig/configuration_test.go @@ -241,12 +241,48 @@ func TestAutomaticApprovalConfig(t *testing.T) { assert.False(t, toolchainCfg.AutomaticApproval().IsEnabled()) }) - t.Run("non-default", func(t *testing.T) { + t.Run("non-default no domains", func(t *testing.T) { cfg := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.AutomaticApproval().Enabled(true)) toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{}) assert.True(t, toolchainCfg.AutomaticApproval().IsEnabled()) }) + + t.Run("non-default single domain", func(t *testing.T) { + cfg := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.AutomaticApproval(). + Enabled(true).Domains("somedomain.org")) + toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{}) + + assert.True(t, toolchainCfg.AutomaticApproval().IsEnabled()) + assert.Equal(t, []string{"somedomain.org"}, toolchainCfg.AutomaticApproval().Domains()) + }) + + t.Run("non-default multiple domains", func(t *testing.T) { + cfg := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.AutomaticApproval(). + Enabled(true).Domains("somedomain.org,otherdomain.edu")) + toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{}) + + assert.True(t, toolchainCfg.AutomaticApproval().IsEnabled()) + assert.Equal(t, []string{"somedomain.org", "otherdomain.edu"}, toolchainCfg.AutomaticApproval().Domains()) + }) + + t.Run("non-default multiple domains with spaces", func(t *testing.T) { + cfg := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.AutomaticApproval(). + Enabled(true).Domains(" somedomain.org, otherdomain.edu ")) + toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{}) + + assert.True(t, toolchainCfg.AutomaticApproval().IsEnabled()) + assert.Equal(t, []string{"somedomain.org", "otherdomain.edu"}, toolchainCfg.AutomaticApproval().Domains()) + }) + + t.Run("non-default empty domains", func(t *testing.T) { + cfg := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.AutomaticApproval(). + Enabled(true).Domains("")) + toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{}) + + assert.True(t, toolchainCfg.AutomaticApproval().IsEnabled()) + assert.Equal(t, len(toolchainCfg.AutomaticApproval().Domains()), 0) + }) } func TestDeactivationConfig(t *testing.T) { diff --git a/controllers/usersignup/approval.go b/controllers/usersignup/approval.go index 76e850ecc..ba2281e48 100644 --- a/controllers/usersignup/approval.go +++ b/controllers/usersignup/approval.go @@ -44,7 +44,8 @@ func stringInList(str string, list []string) bool { return false } -func isAutoApproved(userSignup *toolchainv1alpha1.UserSignup, config toolchainconfig.ToolchainConfig) (bool, error) { +// isAutoApprovalEnabled checks if the auto-approval is enabled for the specific UserSignup +func isAutoApprovalEnabled(userSignup *toolchainv1alpha1.UserSignup, config toolchainconfig.ToolchainConfig) (bool, error) { enabled := config.AutomaticApproval().IsEnabled() domains := config.AutomaticApproval().Domains() email := userSignup.Spec.IdentityClaims.Email @@ -79,7 +80,7 @@ func getClusterIfApproved(ctx context.Context, cl runtimeclient.Client, userSign return false, unknown, errors.Wrapf(err, "unable to get ToolchainConfig") } - autoApproved, err := isAutoApproved(userSignup, config) + autoApproved, err := isAutoApprovalEnabled(userSignup, config) if err != nil { return false, unknown, errors.Wrapf(err, "unable to determine automatic approval") } diff --git a/controllers/usersignup/approval_test.go b/controllers/usersignup/approval_test.go index edde2d8e6..7349e6018 100644 --- a/controllers/usersignup/approval_test.go +++ b/controllers/usersignup/approval_test.go @@ -62,16 +62,15 @@ func TestGetClusterIfApproved(t *testing.T) { assert.Equal(t, "member2", clusterName.getClusterName()) }) - t.Run("with two clusters available, the second one has required cluster-role label", func(t *testing.T) { + t.Run("single cluster, email domain in auto-approved domains", func(t *testing.T) { // given - signup := commonsignup.NewUserSignup() - spc1 := hspc.NewEnabledValidSPC("member1") - spc2 := hspc.NewEnabledValidTenantSPC("member2") + signup := commonsignup.NewUserSignup(commonsignup.WithEmail("tato@somedomain.org")) + spc1 := hspc.NewEnabledValidTenantSPC("member1") toolchainConfig := commonconfig.NewToolchainConfigObjWithReset(t, testconfig.AutomaticApproval(). - Enabled(true), + Enabled(true).Domains("somedomain.org,anotherdomain.edu"), ) - fakeClient := commontest.NewFakeClient(t, toolchainStatus, toolchainConfig, spc1, spc2) + fakeClient := commontest.NewFakeClient(t, toolchainStatus, toolchainConfig, spc1) InitializeCounters(t, toolchainStatus) // when @@ -80,7 +79,68 @@ func TestGetClusterIfApproved(t *testing.T) { // then require.NoError(t, err) assert.True(t, approved) - assert.Equal(t, "member2", clusterName.getClusterName()) + assert.Equal(t, "member1", clusterName.getClusterName()) + }) + + t.Run("single cluster, auto-approved domains is empty", func(t *testing.T) { + // given + signup := commonsignup.NewUserSignup(commonsignup.WithEmail("tato@somedomain.org")) + spc1 := hspc.NewEnabledValidTenantSPC("member1") + toolchainConfig := commonconfig.NewToolchainConfigObjWithReset(t, + testconfig.AutomaticApproval(). + Enabled(true).Domains(""), + ) + fakeClient := commontest.NewFakeClient(t, toolchainStatus, toolchainConfig, spc1) + InitializeCounters(t, toolchainStatus) + + // when + approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient)) + + // then + require.NoError(t, err) + assert.True(t, approved) + assert.Equal(t, "member1", clusterName.getClusterName()) + }) + + + t.Run("single cluster, email domain not in auto-approved domains", func(t *testing.T) { + // given + signup := commonsignup.NewUserSignup(commonsignup.WithEmail("tato@mydomain.org")) + spc1 := hspc.NewEnabledValidTenantSPC("member1") + toolchainConfig := commonconfig.NewToolchainConfigObjWithReset(t, + testconfig.AutomaticApproval(). + Enabled(true).Domains("somedomain.org,anotherdomain.edu"), + ) + fakeClient := commontest.NewFakeClient(t, toolchainStatus, toolchainConfig, spc1) + InitializeCounters(t, toolchainStatus) + + // when + approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient)) + + // then + require.NoError(t, err) + assert.False(t, approved) + assert.Equal(t, unknown, clusterName) + }) + + t.Run("single cluster, invalid email format", func(t *testing.T) { + // given + signup := commonsignup.NewUserSignup(commonsignup.WithEmail("tato@mydomain@somedomain.org")) + spc1 := hspc.NewEnabledValidTenantSPC("member1") + toolchainConfig := commonconfig.NewToolchainConfigObjWithReset(t, + testconfig.AutomaticApproval(). + Enabled(true).Domains("somedomain.org,anotherdomain.edu"), + ) + fakeClient := commontest.NewFakeClient(t, toolchainStatus, toolchainConfig, spc1) + InitializeCounters(t, toolchainStatus) + + // when + approved, clusterName, err := getClusterIfApproved(ctx, fakeClient, signup, capacity.NewClusterManager(test.HostOperatorNs, fakeClient)) + + // then + require.EqualError(t, err, "unable to determine automatic approval: invalid email address: tato@mydomain@somedomain.org") + assert.False(t, approved) + assert.Equal(t, unknown, clusterName) }) t.Run("don't return preferred cluster name if without required cluster role", func(t *testing.T) { diff --git a/go.mod b/go.mod index 4c1245a4c..69b92f3ff 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/codeready-toolchain/host-operator require ( github.com/codeready-toolchain/api v0.0.0-20240425165440-d0a6da0060a5 - github.com/codeready-toolchain/toolchain-common v0.0.0-20240424171146-581ea6502cad + github.com/codeready-toolchain/toolchain-common v0.0.0-20240429093002-78746c952644 github.com/davecgh/go-spew v1.1.1 // indirect github.com/go-bindata/go-bindata v3.1.2+incompatible github.com/go-logr/logr v1.2.3 diff --git a/go.sum b/go.sum index 5aef6cb76..4ef22a52e 100644 --- a/go.sum +++ b/go.sum @@ -142,6 +142,8 @@ github.com/codeready-toolchain/api v0.0.0-20240425165440-d0a6da0060a5 h1:L6NhQrz github.com/codeready-toolchain/api v0.0.0-20240425165440-d0a6da0060a5/go.mod h1:ie9p4LenCCS0LsnbWp6/xwpFDdCWYE0KWzUO6Sk1g0E= github.com/codeready-toolchain/toolchain-common v0.0.0-20240424171146-581ea6502cad h1:ka0mIrFO8+0H4uIgfhgCGh1lmRjGndRUYNEEV8O7fkQ= github.com/codeready-toolchain/toolchain-common v0.0.0-20240424171146-581ea6502cad/go.mod h1:4BqdcAO16K/+YmPbn5XmTKOU4UUyuQPlge7ms9F1NsU= +github.com/codeready-toolchain/toolchain-common v0.0.0-20240429093002-78746c952644 h1:wVAKc4Bj3+tgfKFxAd9NpJ9vn+FCpdOZJXB8rhmtb04= +github.com/codeready-toolchain/toolchain-common v0.0.0-20240429093002-78746c952644/go.mod h1:bluFxwf6L4eWtNlrL3niZaJjFH6UwJnYabls5ay+EQA= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=