diff --git a/dev/config/gitops-config.yaml b/dev/config/gitops-config.yaml index 61ea9fbe88..c7942cff20 100644 --- a/dev/config/gitops-config.yaml +++ b/dev/config/gitops-config.yaml @@ -83,6 +83,3 @@ tenantResources: memory: 100Mi centralRdsCidrBlock: "10.1.0.0/16" - - declarativeConfig: - enabled: true diff --git a/fleetshard/pkg/central/reconciler/argo_reconciler.go b/fleetshard/pkg/central/reconciler/argo_reconciler.go index f0339c13e2..1964eb428d 100644 --- a/fleetshard/pkg/central/reconciler/argo_reconciler.go +++ b/fleetshard/pkg/central/reconciler/argo_reconciler.go @@ -113,7 +113,7 @@ func (r *argoReconciler) makeDesiredArgoCDApplication(remoteCentral private.Mana delete(values, "additionalCAs") } - if isArgoDeclarativeConfigReconciliationEnabled(remoteCentral) { + if r.isArgoDeclarativeConfigReconciliationEnabled(remoteCentral) { dc, _ := values["declarativeConfig"].(map[string]interface{}) if dc == nil { dc = map[string]interface{}{} @@ -177,8 +177,8 @@ func (r *argoReconciler) getSourceRepoURL(m private.ManagedCentral) string { return getTenantResourcesValue(m, "argoCd.sourceRepoUrl", r.argoOpts.TenantDefaultArgoCdAppSourceRepoURL) } -func isArgoDeclarativeConfigReconciliationEnabled(m private.ManagedCentral) bool { - return getTenantResourcesValue(m, "declarativeConfig.enabled", false) +func (r *argoReconciler) isArgoDeclarativeConfigReconciliationEnabled(m private.ManagedCentral) bool { + return getTenantResourcesValue(m, "declarativeConfig.enabled", r.argoOpts.WantsAuthProvider) } func isForceReconcile(m private.ManagedCentral) bool { diff --git a/fleetshard/pkg/central/reconciler/argo_reconciler_test.go b/fleetshard/pkg/central/reconciler/argo_reconciler_test.go index d378f11028..b890aa1568 100644 --- a/fleetshard/pkg/central/reconciler/argo_reconciler_test.go +++ b/fleetshard/pkg/central/reconciler/argo_reconciler_test.go @@ -53,30 +53,38 @@ func TestGetSourceTargetRevision(t *testing.T) { } func TestDeclarativeConfigEnabled(t *testing.T) { - t.Run("returns false for empty ManagedCentral", func(t *testing.T) { - assert.False(t, isArgoDeclarativeConfigReconciliationEnabled(private.ManagedCentral{})) + rWithAuthProvider := argoReconciler{argoOpts: ArgoReconcilerOptions{WantsAuthProvider: true}} + rWithoutAuthProvider := argoReconciler{argoOpts: ArgoReconcilerOptions{WantsAuthProvider: false}} + + t.Run("defaults to WantsAuthProvider for empty ManagedCentral", func(t *testing.T) { + assert.True(t, rWithAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(private.ManagedCentral{})) + assert.False(t, rWithoutAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(private.ManagedCentral{})) }) - t.Run("returns false for nil TenantResourcesValues", func(t *testing.T) { - assert.False(t, isArgoDeclarativeConfigReconciliationEnabled(private.ManagedCentral{ + t.Run("defaults to WantsAuthProvider for nil TenantResourcesValues", func(t *testing.T) { + mc := private.ManagedCentral{ Spec: private.ManagedCentralAllOfSpec{ TenantResourcesValues: nil, }, - })) + } + assert.True(t, rWithAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) + assert.False(t, rWithoutAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) }) - t.Run("returns false when declarativeConfig is missing", func(t *testing.T) { - assert.False(t, isArgoDeclarativeConfigReconciliationEnabled(private.ManagedCentral{ + t.Run("defaults to WantsAuthProvider when declarativeConfig is missing", func(t *testing.T) { + mc := private.ManagedCentral{ Spec: private.ManagedCentralAllOfSpec{ TenantResourcesValues: map[string]interface{}{ "other": "value", }, }, - })) + } + assert.True(t, rWithAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) + assert.False(t, rWithoutAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) }) t.Run("returns false when explicitly disabled", func(t *testing.T) { - assert.False(t, isArgoDeclarativeConfigReconciliationEnabled(private.ManagedCentral{ + mc := private.ManagedCentral{ Spec: private.ManagedCentralAllOfSpec{ TenantResourcesValues: map[string]interface{}{ "declarativeConfig": map[string]interface{}{ @@ -84,11 +92,13 @@ func TestDeclarativeConfigEnabled(t *testing.T) { }, }, }, - })) + } + assert.False(t, rWithAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) + assert.False(t, rWithoutAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) }) - t.Run("returns false when enabled is wrong type", func(t *testing.T) { - assert.False(t, isArgoDeclarativeConfigReconciliationEnabled(private.ManagedCentral{ + t.Run("defaults to WantsAuthProvider when enabled is wrong type", func(t *testing.T) { + mc := private.ManagedCentral{ Spec: private.ManagedCentralAllOfSpec{ TenantResourcesValues: map[string]interface{}{ "declarativeConfig": map[string]interface{}{ @@ -96,11 +106,13 @@ func TestDeclarativeConfigEnabled(t *testing.T) { }, }, }, - })) + } + assert.True(t, rWithAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) + assert.False(t, rWithoutAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) }) t.Run("returns true when enabled", func(t *testing.T) { - assert.True(t, isArgoDeclarativeConfigReconciliationEnabled(private.ManagedCentral{ + mc := private.ManagedCentral{ Spec: private.ManagedCentralAllOfSpec{ TenantResourcesValues: map[string]interface{}{ "declarativeConfig": map[string]interface{}{ @@ -108,7 +120,9 @@ func TestDeclarativeConfigEnabled(t *testing.T) { }, }, }, - })) + } + assert.True(t, rWithAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) + assert.True(t, rWithoutAuthProvider.isArgoDeclarativeConfigReconciliationEnabled(mc)) }) } diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 780b63a488..b643051ceb 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -15,7 +15,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/golang/glog" - "github.com/hashicorp/go-multierror" openshiftRouteV1 "github.com/openshift/api/route/v1" "github.com/pkg/errors" "github.com/stackrox/acs-fleet-manager/fleetshard/config" @@ -28,7 +27,6 @@ import ( "github.com/stackrox/acs-fleet-manager/internal/central/pkg/api/private" "github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager" centralNotifierUtils "github.com/stackrox/rox/central/notifiers/utils" - "github.com/stackrox/rox/pkg/declarativeconfig" "github.com/stackrox/rox/pkg/random" "golang.org/x/exp/maps" "gopkg.in/yaml.v2" @@ -66,10 +64,6 @@ const ( centralExpiredAtKey = "rhacs.redhat.com/expired-at" - auditLogNotifierKey = "com.redhat.rhacs.auditLogNotifier" - auditLogNotifierName = "Platform Audit Logs" - auditLogTenantIDKey = "tenant_id" - dbUserTypeAnnotation = "platform.stackrox.io/user-type" dbUserTypeMaster = "master" dbUserTypeCentral = "central" @@ -79,18 +73,11 @@ const ( centralDbOverrideConfigMap = "central-db-override" centralDeletePollInterval = 5 * time.Second - centralEncryptionKeySecretName = "central-encryption-key-chain" // pragma: allowlist secret - - sensibleDeclarativeConfigSecretName = "cloud-service-sensible-declarative-configs" // pragma: allowlist secret - authProviderClientCredentialsSecretName = "default-auth-provider-client-credentials" // pragma: allowlist secret - - authProviderDeclarativeConfigKey = "default-sso-auth-provider" - additionalAuthProviderConfigKey = "additional-auth-provider" - - tenantImagePullSecretName = "stackrox" // pragma: allowlist secret + centralEncryptionKeySecretName = "central-encryption-key-chain" // pragma: allowlist secret + authProviderClientCredentialsSecretName = "default-auth-provider-client-credentials" // pragma: allowlist secret + tenantImagePullSecretName = "stackrox" // pragma: allowlist secret ) -type verifyAuthProviderExistsFunc func(ctx context.Context, central private.ManagedCentral, client ctrlClient.Client) (bool, error) type needsReconcileFunc func(changed bool, central private.ManagedCentral, storedSecrets []string) bool type restoreCentralSecretsFunc func(ctx context.Context, remoteCentral private.ManagedCentral) error type areSecretsStoredFunc func(secretsStored []string) bool @@ -103,7 +90,6 @@ type encryptedSecrets struct { // CentralReconcilerOptions are the static options for creating a reconciler. type CentralReconcilerOptions struct { UseRoutes bool - WantsAuthProvider bool ManagedDBEnabled bool ClusterName string Environment string @@ -136,7 +122,6 @@ type CentralReconciler struct { managedDbReconciler *managedDbReconciler managedDBEnabled bool - wantsAuthProvider bool tenantImagePullSecret []byte clock clock @@ -318,166 +303,23 @@ func (r *CentralReconciler) reconcileInstanceDeletion(ctx context.Context, remot return nil, ErrDeletionInProgress } -func getAuditLogNotifierConfig( - auditLoggingConfig config.AuditLogging, - namespace string, -) *declarativeconfig.Notifier { - return &declarativeconfig.Notifier{ - Name: auditLogNotifierName, - GenericConfig: &declarativeconfig.GenericConfig{ - Endpoint: auditLoggingConfig.Endpoint(true), - SkipTLSVerify: auditLoggingConfig.SkipTLSVerify, - AuditLoggingEnabled: true, - ExtraFields: []declarativeconfig.KeyValuePair{ - { - Key: auditLogTenantIDKey, - Value: namespace, - }, - }, - }, - } -} - -func (r *CentralReconciler) configureAuditLogNotifier(secret *corev1.Secret, namespace string) error { - if !r.auditLogging.Enabled { - return nil - } - if secret.Data == nil { - secret.Data = make(map[string][]byte) - } - auditLogNotifierConfig := getAuditLogNotifierConfig( - r.auditLogging, - namespace, - ) - encodedNotifierConfig, marshalErr := yaml.Marshal(auditLogNotifierConfig) - if marshalErr != nil { - return fmt.Errorf("marshaling audit log notifier configuration: %w", marshalErr) - } - secret.Data[auditLogNotifierKey] = encodedNotifierConfig - return nil -} - -func getAuthProviderConfig(remoteCentral private.ManagedCentral) *declarativeconfig.AuthProvider { - groups := []declarativeconfig.Group{ - { - AttributeKey: "userid", - AttributeValue: remoteCentral.Spec.Auth.OwnerUserId, - RoleName: "Admin", - }, - { - AttributeKey: "groups", - AttributeValue: "admin:org:all", - RoleName: "Admin", - }, - { - AttributeKey: "rh_is_org_admin", - AttributeValue: "true", - RoleName: "Admin", - }, - } - if remoteCentral.Spec.Auth.OwnerAlternateUserId != "" { - groups = append(groups, declarativeconfig.Group{ - AttributeKey: "userid", - AttributeValue: remoteCentral.Spec.Auth.OwnerAlternateUserId, - RoleName: "Admin", - }) - } - return &declarativeconfig.AuthProvider{ - Name: authProviderName(remoteCentral), - UIEndpoint: remoteCentral.Spec.UiHost, - ExtraUIEndpoints: []string{"localhost:8443"}, - Groups: groups, - RequiredAttributes: []declarativeconfig.RequiredAttribute{ - { - AttributeKey: "rh_org_id", - AttributeValue: remoteCentral.Spec.Auth.OwnerOrgId, - }, - }, - ClaimMappings: []declarativeconfig.ClaimMapping{ - { - Path: "realm_access.roles", - Name: "groups", - }, - { - Path: "org_id", - Name: "rh_org_id", - }, - { - Path: "is_org_admin", - Name: "rh_is_org_admin", - }, - { - Path: "deprecated_sub", - Name: "userid", - }, - }, - OIDCConfig: &declarativeconfig.OIDCConfig{ - Issuer: remoteCentral.Spec.Auth.Issuer, - CallbackMode: "post", - ClientID: remoteCentral.Spec.Auth.ClientId, - ClientSecret: remoteCentral.Spec.Auth.ClientSecret, // pragma: allowlist secret - DisableOfflineAccessScope: true, - }, - } -} - -func (r *CentralReconciler) configureAuthProvider(secret *corev1.Secret, remoteCentral private.ManagedCentral) error { - if !r.wantsAuthProvider { - return nil - } - - if secret.Data == nil { - secret.Data = make(map[string][]byte) - } - - authProviderConfig := getAuthProviderConfig(remoteCentral) - - rawAuthProviderBytes, err := yaml.Marshal(authProviderConfig) - if err != nil { - return fmt.Errorf("marshaling auth provider configuration: %w", err) - } - secret.Data[authProviderDeclarativeConfigKey] = rawAuthProviderBytes - return nil -} - func (r *CentralReconciler) reconcileDeclarativeConfigurationData(ctx context.Context, remoteCentral private.ManagedCentral) error { - if isArgoDeclarativeConfigReconciliationEnabled(remoteCentral) { - return ensureSecretExists( - ctx, - r.client, - remoteCentral.Metadata.Namespace, - authProviderClientCredentialsSecretName, - func(secret *corev1.Secret) error { - if secret.Data == nil { - secret.Data = make(map[string][]byte) - } - secret.Data["clientID"] = []byte(remoteCentral.Spec.Auth.ClientId) - secret.Data["clientSecret"] = []byte(remoteCentral.Spec.Auth.ClientSecret) // pragma: allowlist secret - return nil - }, - ) + if !r.argoReconciler.isArgoDeclarativeConfigReconciliationEnabled(remoteCentral) { + return nil } - namespace := remoteCentral.Metadata.Namespace return ensureSecretExists( ctx, r.client, - namespace, - sensibleDeclarativeConfigSecretName, + remoteCentral.Metadata.Namespace, + authProviderClientCredentialsSecretName, func(secret *corev1.Secret) error { - var configErrs *multierror.Error - if err := r.configureAuditLogNotifier(secret, namespace); err != nil { - configErrs = multierror.Append(configErrs, err) - } - if err := r.configureAuthProvider(secret, remoteCentral); err != nil { - configErrs = multierror.Append(configErrs, err) + if secret.Data == nil { + secret.Data = make(map[string][]byte) } - if err := r.configureAdditionalAuthProvider(secret, remoteCentral); err != nil { - configErrs = multierror.Append(configErrs, err) - } - return errors.Wrapf(configErrs.ErrorOrNil(), - "configuring declarative configurations within secret %s/%s", - secret.GetNamespace(), secret.GetName()) + secret.Data["clientID"] = []byte(remoteCentral.Spec.Auth.ClientId) + secret.Data["clientSecret"] = []byte(remoteCentral.Spec.Auth.ClientSecret) // pragma: allowlist secret + return nil }, ) } @@ -985,71 +827,6 @@ func (r *CentralReconciler) needsReconcile(changed bool, remoteCentral private.M return false } -func (r *CentralReconciler) configureAdditionalAuthProvider(secret *corev1.Secret, central private.ManagedCentral) error { - authProviderConfig := findAdditionalAuthProvider(central) - if authProviderConfig == nil { - return nil - } - if secret.Data == nil { - secret.Data = make(map[string][]byte) - } - - rawAuthProviderBytes, err := yaml.Marshal(authProviderConfig) - if err != nil { - return fmt.Errorf("marshaling additional auth provider configuration: %w", err) - } - secret.Data[additionalAuthProviderConfigKey] = rawAuthProviderBytes - return nil -} - -func findAdditionalAuthProvider(central private.ManagedCentral) *declarativeconfig.AuthProvider { - authProvider := central.Spec.AdditionalAuthProvider - // Assume that if name is not specified, no additional auth provider is configured. - if authProvider.Name == "" { - return nil - } - groups := make([]declarativeconfig.Group, 0, len(authProvider.Groups)) - for _, group := range authProvider.Groups { - groups = append(groups, declarativeconfig.Group{ - AttributeKey: group.Key, - AttributeValue: group.Value, - RoleName: group.Role, - }) - } - - requiredAttributes := make([]declarativeconfig.RequiredAttribute, 0, len(authProvider.RequiredAttributes)) - for _, requiredAttribute := range authProvider.RequiredAttributes { - requiredAttributes = append(requiredAttributes, declarativeconfig.RequiredAttribute{ - AttributeKey: requiredAttribute.Key, - AttributeValue: requiredAttribute.Value, - }) - } - - claimMappings := make([]declarativeconfig.ClaimMapping, 0, len(authProvider.ClaimMappings)) - for _, claimMapping := range authProvider.ClaimMappings { - claimMappings = append(claimMappings, declarativeconfig.ClaimMapping{ - Path: claimMapping.Key, - Name: claimMapping.Value, - }) - } - - return &declarativeconfig.AuthProvider{ - Name: authProvider.Name, - UIEndpoint: central.Spec.UiHost, - ExtraUIEndpoints: []string{"localhost:8443"}, - Groups: groups, - RequiredAttributes: requiredAttributes, - ClaimMappings: claimMappings, - OIDCConfig: &declarativeconfig.OIDCConfig{ - Issuer: authProvider.Oidc.Issuer, - CallbackMode: authProvider.Oidc.CallbackMode, - ClientID: authProvider.Oidc.ClientID, - ClientSecret: authProvider.Oidc.ClientSecret, // pragma: allowlist secret - DisableOfflineAccessScope: authProvider.Oidc.DisableOfflineAccessScope, - }, - } -} - // NewCentralReconciler ... func NewCentralReconciler(k8sClient ctrlClient.Client, fleetmanagerClient *fleetmanager.Client, managedDBProvisioningClient cloudprovider.DBClient, managedDBInitFunc postgres.CentralDBInitFunc, @@ -1065,7 +842,6 @@ func NewCentralReconciler(k8sClient ctrlClient.Client, fleetmanagerClient *fleet fleetmanagerClient: fleetmanagerClient, status: pointer.Int32(FreeStatus), useRoutes: opts.UseRoutes, - wantsAuthProvider: opts.WantsAuthProvider, namespaceReconciler: nsReconciler, argoReconciler: argoReconciler, tenantCleanup: NewTenantCleanup(k8sClient, tenantCleanupOptions), diff --git a/fleetshard/pkg/central/reconciler/reconciler_test.go b/fleetshard/pkg/central/reconciler/reconciler_test.go index 74dcc601d6..619ce0b7af 100644 --- a/fleetshard/pkg/central/reconciler/reconciler_test.go +++ b/fleetshard/pkg/central/reconciler/reconciler_test.go @@ -27,7 +27,6 @@ import ( "github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager" fmMocks "github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager/mocks" centralNotifierUtils "github.com/stackrox/rox/central/notifiers/utils" - "github.com/stackrox/rox/pkg/declarativeconfig" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" @@ -67,30 +66,6 @@ var ( ArgoCdNamespace: "openshift-gitops", }, } - - defaultAuditLogConfig = config.AuditLogging{ - Enabled: true, - URLScheme: "https", - AuditLogTargetHost: "audit-logs-aggregator.rhacs-audit-logs", - AuditLogTargetPort: 8888, - SkipTLSVerify: true, - } - - vectorAuditLogConfig = config.AuditLogging{ - Enabled: true, - URLScheme: "https", - AuditLogTargetHost: "rhacs-vector.rhacs", - AuditLogTargetPort: 8443, - SkipTLSVerify: true, - } - - disabledAuditLogConfig = config.AuditLogging{ - Enabled: false, - URLScheme: "https", - AuditLogTargetHost: "audit-logs-aggregator.rhacs-audit-logs", - AuditLogTargetPort: 8888, - SkipTLSVerify: false, - } ) var simpleManagedCentral = private.ManagedCentral{ @@ -1048,213 +1023,51 @@ func TestEnsureSecretExists(t *testing.T) { }) } -func TestGetAuditLogNotifierConfig(t *testing.T) { - testCases := []struct { - namespace string - auditLogging config.AuditLogging - auditLogTarget string - auditLogPort int - expectedConfig *declarativeconfig.Notifier - }{ - { - namespace: centralNamespace, - auditLogging: defaultAuditLogConfig, - auditLogTarget: defaultAuditLogConfig.AuditLogTargetHost, - auditLogPort: defaultAuditLogConfig.AuditLogTargetPort, - expectedConfig: &declarativeconfig.Notifier{ - Name: auditLogNotifierName, - GenericConfig: &declarativeconfig.GenericConfig{ - Endpoint: fmt.Sprintf( - "https://%s:%d", - defaultAuditLogConfig.AuditLogTargetHost, - defaultAuditLogConfig.AuditLogTargetPort, - ), - SkipTLSVerify: true, - AuditLoggingEnabled: true, - ExtraFields: []declarativeconfig.KeyValuePair{ - { - Key: auditLogTenantIDKey, - Value: centralNamespace, - }, - }, - }, - }, - }, - { - namespace: "rhacs", - auditLogging: vectorAuditLogConfig, - auditLogTarget: vectorAuditLogConfig.AuditLogTargetHost, - auditLogPort: vectorAuditLogConfig.AuditLogTargetPort, - expectedConfig: &declarativeconfig.Notifier{ - Name: auditLogNotifierName, - GenericConfig: &declarativeconfig.GenericConfig{ - Endpoint: fmt.Sprintf( - "https://%s:%d", - vectorAuditLogConfig.AuditLogTargetHost, - vectorAuditLogConfig.AuditLogTargetPort, - ), - SkipTLSVerify: true, - AuditLoggingEnabled: true, - ExtraFields: []declarativeconfig.KeyValuePair{ - { - Key: auditLogTenantIDKey, - Value: "rhacs", - }, - }, - }, - }, - }, - } - for _, testCase := range testCases { - notifierConfig := getAuditLogNotifierConfig(testCase.auditLogging, testCase.namespace) - assert.Equal(t, testCase.expectedConfig, notifierConfig) - } -} - -func populateDeclarativeConfigSecrets( - t *testing.T, - namespace string, - payload map[string][]declarativeconfig.Configuration, -) *v1.Secret { - if len(payload) == 0 { - return getSecret(sensibleDeclarativeConfigSecretName, namespace, nil) - } - secretData := make(map[string][]byte, len(payload)) - for dataKey, configList := range payload { - switch len(configList) { - case 0: - secretData[dataKey] = nil - case 1: - encodedBytes, encodingErr := yaml.Marshal(configList[0]) - assert.NoError(t, encodingErr) - secretData[dataKey] = encodedBytes - default: - encodedBytes, encodingErr := yaml.Marshal(configList) - assert.NoError(t, encodingErr) - secretData[dataKey] = encodedBytes - } - } - return getSecret(sensibleDeclarativeConfigSecretName, centralNamespace, secretData) -} - func TestReconcileDeclarativeConfigurationData(t *testing.T) { - defaultNotifierConfig := getAuditLogNotifierConfig( - defaultAuditLogConfig, - centralNamespace, - ) - faultyVectorNotifierConfig := getAuditLogNotifierConfig( - vectorAuditLogConfig, - "rhacs", - ) - correctVectorNotifierConfig := getAuditLogNotifierConfig( - vectorAuditLogConfig, - centralNamespace, - ) + t.Run("creates auth provider client credentials secret when declarative config enabled", func(t *testing.T) { + opts := defaultReconcilerOptions + opts.ArgoReconcilerOptions.WantsAuthProvider = true + fakeClient, _, r := getClientTrackerAndReconciler(t, nil, opts) - authProviderConfig := getAuthProviderConfig(simpleManagedCentral) + managedCentral := simpleManagedCentral + managedCentral.Spec.Auth.ClientId = "my-client-id" + managedCentral.Spec.Auth.ClientSecret = "my-client-secret" // pragma: allowlist secret - const otherItemKey = "other.item.key" + ctx := context.TODO() + require.NoError(t, r.reconcileDeclarativeConfigurationData(ctx, managedCentral)) - testCases := []struct { - name string - auditLogConfig config.AuditLogging - preExistingSecret bool - initialDeclarativeConfigs map[string][]declarativeconfig.Configuration - expectedDeclarativeConfigs map[string][]declarativeconfig.Configuration - wantsAuthProvider bool - }{ - { - name: "Missing default secret gets created", - auditLogConfig: defaultAuditLogConfig, - preExistingSecret: false, // pragma: allowlist secret - expectedDeclarativeConfigs: map[string][]declarativeconfig.Configuration{ - auditLogNotifierKey: {defaultNotifierConfig}, - authProviderDeclarativeConfigKey: {authProviderConfig}, - }, - wantsAuthProvider: true, - }, - { - name: "Missing vector secret gets created", - auditLogConfig: vectorAuditLogConfig, - preExistingSecret: false, // pragma: allowlist secret - expectedDeclarativeConfigs: map[string][]declarativeconfig.Configuration{ - auditLogNotifierKey: {correctVectorNotifierConfig}, - }, - }, - { - name: "Empty secret when audit logging and auth provider creation disabled", - auditLogConfig: disabledAuditLogConfig, - preExistingSecret: false, // pragma: allowlist secret - }, - { - name: "No secret modification when audit logging and auth provider creation disabled", - auditLogConfig: disabledAuditLogConfig, - preExistingSecret: true, // pragma: allowlist secret - initialDeclarativeConfigs: map[string][]declarativeconfig.Configuration{ - auditLogNotifierKey: {defaultNotifierConfig}, - authProviderDeclarativeConfigKey: {authProviderConfig}, - otherItemKey: {faultyVectorNotifierConfig}, - }, - expectedDeclarativeConfigs: map[string][]declarativeconfig.Configuration{ - auditLogNotifierKey: {defaultNotifierConfig}, - authProviderDeclarativeConfigKey: {authProviderConfig}, - otherItemKey: {faultyVectorNotifierConfig}, - }, - }, - } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - ctx := context.TODO() - reconcilerOptions := CentralReconcilerOptions{ - AuditLogging: testCase.auditLogConfig, - WantsAuthProvider: testCase.wantsAuthProvider, - } - fakeClient, _, r := getClientTrackerAndReconciler( - t, - nil, - reconcilerOptions, - ) - if testCase.preExistingSecret { - secret := populateDeclarativeConfigSecrets(t, centralNamespace, testCase.initialDeclarativeConfigs) - require.NoError(t, fakeClient.Create(ctx, secret)) - } - assert.NoError(t, r.reconcileDeclarativeConfigurationData(ctx, simpleManagedCentral)) - fetchedSecret := &v1.Secret{} - secretKey := client.ObjectKey{ // pragma: allowlist secret - Name: sensibleDeclarativeConfigSecretName, - Namespace: centralNamespace, - } - postFetchErr := fakeClient.Get(ctx, secretKey, fetchedSecret) - assert.NoError(t, postFetchErr) - expectedSecret := populateDeclarativeConfigSecrets(t, centralNamespace, testCase.expectedDeclarativeConfigs) - compareSecret(t, expectedSecret, fetchedSecret, !testCase.preExistingSecret) - }) - } -} + secret := &v1.Secret{} + err := fakeClient.Get(ctx, client.ObjectKey{ + Namespace: centralNamespace, + Name: authProviderClientCredentialsSecretName, + }, secret) + require.NoError(t, err) + assert.Equal(t, []byte("my-client-id"), secret.Data["clientID"]) + assert.Equal(t, []byte("my-client-secret"), secret.Data["clientSecret"]) + }) -func TestReconcileDeclarativeConfigSkippedWhenArgoManaged(t *testing.T) { - centralWithDeclarativeConfig := simpleManagedCentral - centralWithDeclarativeConfig.Spec.TenantResourcesValues = map[string]interface{}{ - "declarativeConfig": map[string]interface{}{ - "enabled": true, - }, - } + t.Run("skips secret creation when declarative config disabled", func(t *testing.T) { + opts := defaultReconcilerOptions + opts.ArgoReconcilerOptions.WantsAuthProvider = false + fakeClient, _, r := getClientTrackerAndReconciler(t, nil, opts) - reconcilerOptions := CentralReconcilerOptions{ - WantsAuthProvider: true, - AuditLogging: defaultAuditLogConfig, - } - fakeClient, _, r := getClientTrackerAndReconciler(t, nil, reconcilerOptions) + managedCentral := simpleManagedCentral + managedCentral.Spec.TenantResourcesValues = map[string]interface{}{ + "declarativeConfig": map[string]interface{}{ + "enabled": false, + }, + } - err := r.reconcileDeclarativeConfigurationData(context.Background(), centralWithDeclarativeConfig) - require.NoError(t, err) + ctx := context.TODO() + require.NoError(t, r.reconcileDeclarativeConfigurationData(ctx, managedCentral)) - secret := &v1.Secret{} - getErr := fakeClient.Get(context.Background(), client.ObjectKey{ - Namespace: centralNamespace, - Name: sensibleDeclarativeConfigSecretName, - }, secret) - assert.True(t, k8sErrors.IsNotFound(getErr), "secret should not be created when declarativeConfig.enabled is true") + secret := &v1.Secret{} + err := fakeClient.Get(ctx, client.ObjectKey{ + Namespace: centralNamespace, + Name: authProviderClientCredentialsSecretName, + }, secret) + assert.True(t, k8sErrors.IsNotFound(err)) + }) } func TestRestoreCentralSecrets(t *testing.T) { diff --git a/fleetshard/pkg/central/reconciler/util.go b/fleetshard/pkg/central/reconciler/util.go index ed57211e0c..bfe95c6835 100644 --- a/fleetshard/pkg/central/reconciler/util.go +++ b/fleetshard/pkg/central/reconciler/util.go @@ -3,25 +3,20 @@ package reconciler import ( "context" "fmt" - "strings" "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/k8s" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/stackrox/rox/pkg/urlfmt" appsv1 "k8s.io/api/apps/v1" apiErrors "k8s.io/apimachinery/pkg/api/errors" "github.com/pkg/errors" - "github.com/stackrox/acs-fleet-manager/internal/central/pkg/api/private" core "k8s.io/api/core/v1" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" ) const ( centralDeploymentName = "central" - centralServiceName = "central" - oidcType = "oidc" ) func isCentralDeploymentReady(ctx context.Context, client ctrlClient.Client, namespace string) (bool, error) { @@ -38,48 +33,6 @@ func isCentralDeploymentReady(ctx context.Context, client ctrlClient.Client, nam return deployment.Status.AvailableReplicas > 0 && deployment.Status.UnavailableReplicas == 0, nil } -// TODO: ROX-11644: doesn't work when fleetshard-sync deployed outside of Central's cluster -func getServiceAddress(ctx context.Context, namespace string, client ctrlClient.Client) (string, error) { - service := &core.Service{} - err := client.Get(ctx, - ctrlClient.ObjectKey{Name: centralServiceName, Namespace: namespace}, - service) - if err != nil { - return "", errors.Wrapf(err, "getting k8s service for central") - } - port, err := getHTTPSServicePort(service) - if err != nil { - return "", err - } - address := fmt.Sprintf("https://%s.%s.svc.cluster.local:%d", centralServiceName, namespace, port) - return address, nil -} - -func getHTTPSServicePort(service *core.Service) (int32, error) { - for _, servicePort := range service.Spec.Ports { - if servicePort.Name == "https" { - return servicePort.Port, nil - } - } - return 0, errors.Errorf("no `https` port is present in %s/%s service", service.Namespace, service.Name) -} - -// authProviderName deduces auth provider name from issuer URL. -func authProviderName(central private.ManagedCentral) (name string) { - switch { - case strings.Contains(central.Spec.Auth.Issuer, "sso.stage.redhat"): - name = "Red Hat SSO (stage)" - case strings.Contains(central.Spec.Auth.Issuer, "sso.redhat"): - name = "Red Hat SSO" - default: - name = urlfmt.GetServerFromURL(central.Spec.Auth.Issuer) - } - if name == "" { - name = "SSO" - } - return -} - func checkSecretExists( ctx context.Context, client ctrlClient.Client, diff --git a/fleetshard/pkg/runtime/runtime.go b/fleetshard/pkg/runtime/runtime.go index 69d3745c80..76421c9060 100644 --- a/fleetshard/pkg/runtime/runtime.go +++ b/fleetshard/pkg/runtime/runtime.go @@ -131,7 +131,6 @@ func (r *Runtime) Start(ctx context.Context) error { reconcilerOpts := centralReconciler.CentralReconcilerOptions{ UseRoutes: routesAvailable, - WantsAuthProvider: r.config.CreateAuthProvider, ManagedDBEnabled: r.config.ManagedDB.Enabled, ClusterName: r.config.ClusterName, Environment: r.config.Environment, diff --git a/internal/central/pkg/api/private/api/openapi.yaml b/internal/central/pkg/api/private/api/openapi.yaml index b01666ac22..9fe2567f2a 100644 --- a/internal/central/pkg/api/private/api/openapi.yaml +++ b/internal/central/pkg/api/private/api/openapi.yaml @@ -352,52 +352,6 @@ components: type: string issuer: type: string - ManagedCentral_allOf_spec_additionalAuthProvider_groups: - properties: - key: - type: string - value: - type: string - role: - type: string - ManagedCentral_allOf_spec_additionalAuthProvider_requiredAttributes: - properties: - key: - type: string - value: - type: string - ManagedCentral_allOf_spec_additionalAuthProvider_oidc: - properties: - issuer: - type: string - callbackMode: - type: string - clientID: - type: string - clientSecret: - type: string - disableOfflineAccessScope: - type: boolean - ManagedCentral_allOf_spec_additionalAuthProvider: - properties: - name: - type: string - minimumRoleName: - type: string - groups: - items: - $ref: '#/components/schemas/ManagedCentral_allOf_spec_additionalAuthProvider_groups' - type: array - requiredAttributes: - items: - $ref: '#/components/schemas/ManagedCentral_allOf_spec_additionalAuthProvider_requiredAttributes' - type: array - claimMappings: - items: - $ref: '#/components/schemas/ManagedCentral_allOf_spec_additionalAuthProvider_requiredAttributes' - type: array - oidc: - $ref: '#/components/schemas/ManagedCentral_allOf_spec_additionalAuthProvider_oidc' ManagedCentral_allOf_spec: properties: instanceType: @@ -413,8 +367,6 @@ components: type: array auth: $ref: '#/components/schemas/ManagedCentral_allOf_spec_auth' - additionalAuthProvider: - $ref: '#/components/schemas/ManagedCentral_allOf_spec_additionalAuthProvider' uiHost: description: Handles GUI/CLI/API connections type: string diff --git a/internal/central/pkg/api/private/model_managed_central_all_of_spec.go b/internal/central/pkg/api/private/model_managed_central_all_of_spec.go index 71f7941ecf..d26f21e4cd 100644 --- a/internal/central/pkg/api/private/model_managed_central_all_of_spec.go +++ b/internal/central/pkg/api/private/model_managed_central_all_of_spec.go @@ -12,11 +12,10 @@ package private // ManagedCentralAllOfSpec struct for ManagedCentralAllOfSpec type ManagedCentralAllOfSpec struct { - InstanceType string `json:"instanceType,omitempty"` - TenantResourcesValues map[string]interface{} `json:"tenantResourcesValues,omitempty"` - Owners []string `json:"owners,omitempty"` - Auth ManagedCentralAllOfSpecAuth `json:"auth,omitempty"` - AdditionalAuthProvider ManagedCentralAllOfSpecAdditionalAuthProvider `json:"additionalAuthProvider,omitempty"` + InstanceType string `json:"instanceType,omitempty"` + TenantResourcesValues map[string]interface{} `json:"tenantResourcesValues,omitempty"` + Owners []string `json:"owners,omitempty"` + Auth ManagedCentralAllOfSpecAuth `json:"auth,omitempty"` // Handles GUI/CLI/API connections UiHost string `json:"uiHost,omitempty"` // Handles Sensor connections diff --git a/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider.go b/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider.go deleted file mode 100644 index 771eec7c42..0000000000 --- a/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider.go +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Red Hat Advanced Cluster Security Service Fleet Manager - * - * Red Hat Advanced Cluster Security (RHACS) Service Fleet Manager APIs that are used by internal services e.g fleetshard-sync. - * - * API version: 1.4.0 - * Generated by: OpenAPI Generator (https://openapi-generator.tech) - */ - -// Code generated by OpenAPI Generator (https://openapi-generator.tech). DO NOT EDIT. -package private - -// ManagedCentralAllOfSpecAdditionalAuthProvider struct for ManagedCentralAllOfSpecAdditionalAuthProvider -type ManagedCentralAllOfSpecAdditionalAuthProvider struct { - Name string `json:"name,omitempty"` - MinimumRoleName string `json:"minimumRoleName,omitempty"` - Groups []ManagedCentralAllOfSpecAdditionalAuthProviderGroups `json:"groups,omitempty"` - RequiredAttributes []ManagedCentralAllOfSpecAdditionalAuthProviderRequiredAttributes `json:"requiredAttributes,omitempty"` - ClaimMappings []ManagedCentralAllOfSpecAdditionalAuthProviderRequiredAttributes `json:"claimMappings,omitempty"` - Oidc ManagedCentralAllOfSpecAdditionalAuthProviderOidc `json:"oidc,omitempty"` -} diff --git a/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider_groups.go b/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider_groups.go deleted file mode 100644 index 775905859b..0000000000 --- a/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider_groups.go +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Red Hat Advanced Cluster Security Service Fleet Manager - * - * Red Hat Advanced Cluster Security (RHACS) Service Fleet Manager APIs that are used by internal services e.g fleetshard-sync. - * - * API version: 1.4.0 - * Generated by: OpenAPI Generator (https://openapi-generator.tech) - */ - -// Code generated by OpenAPI Generator (https://openapi-generator.tech). DO NOT EDIT. -package private - -// ManagedCentralAllOfSpecAdditionalAuthProviderGroups struct for ManagedCentralAllOfSpecAdditionalAuthProviderGroups -type ManagedCentralAllOfSpecAdditionalAuthProviderGroups struct { - Key string `json:"key,omitempty"` - Value string `json:"value,omitempty"` - Role string `json:"role,omitempty"` -} diff --git a/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider_oidc.go b/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider_oidc.go deleted file mode 100644 index cbc951750d..0000000000 --- a/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider_oidc.go +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Red Hat Advanced Cluster Security Service Fleet Manager - * - * Red Hat Advanced Cluster Security (RHACS) Service Fleet Manager APIs that are used by internal services e.g fleetshard-sync. - * - * API version: 1.4.0 - * Generated by: OpenAPI Generator (https://openapi-generator.tech) - */ - -// Code generated by OpenAPI Generator (https://openapi-generator.tech). DO NOT EDIT. -package private - -// ManagedCentralAllOfSpecAdditionalAuthProviderOidc struct for ManagedCentralAllOfSpecAdditionalAuthProviderOidc -type ManagedCentralAllOfSpecAdditionalAuthProviderOidc struct { - Issuer string `json:"issuer,omitempty"` - CallbackMode string `json:"callbackMode,omitempty"` - ClientID string `json:"clientID,omitempty"` - ClientSecret string `json:"clientSecret,omitempty"` - DisableOfflineAccessScope bool `json:"disableOfflineAccessScope,omitempty"` -} diff --git a/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider_required_attributes.go b/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider_required_attributes.go deleted file mode 100644 index 1008a00eb4..0000000000 --- a/internal/central/pkg/api/private/model_managed_central_all_of_spec_additional_auth_provider_required_attributes.go +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Red Hat Advanced Cluster Security Service Fleet Manager - * - * Red Hat Advanced Cluster Security (RHACS) Service Fleet Manager APIs that are used by internal services e.g fleetshard-sync. - * - * API version: 1.4.0 - * Generated by: OpenAPI Generator (https://openapi-generator.tech) - */ - -// Code generated by OpenAPI Generator (https://openapi-generator.tech). DO NOT EDIT. -package private - -// ManagedCentralAllOfSpecAdditionalAuthProviderRequiredAttributes struct for ManagedCentralAllOfSpecAdditionalAuthProviderRequiredAttributes -type ManagedCentralAllOfSpecAdditionalAuthProviderRequiredAttributes struct { - Key string `json:"key,omitempty"` - Value string `json:"value,omitempty"` -} diff --git a/internal/central/pkg/gitops/config.go b/internal/central/pkg/gitops/config.go index 09193027dc..55336c0645 100644 --- a/internal/central/pkg/gitops/config.go +++ b/internal/central/pkg/gitops/config.go @@ -2,8 +2,6 @@ package gitops import ( - "fmt" - argocd "github.com/stackrox/acs-fleet-manager/pkg/argocd/apis/application/v1alpha1" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -11,7 +9,6 @@ import ( // Config represents the gitops configuration type Config struct { TenantResources TenantResourceConfig `json:"tenantResources"` - Centrals CentralsConfig `json:"centrals"` Applications []argocd.Application `json:"applications"` } @@ -62,11 +59,6 @@ type AuthProviderOIDCConfig struct { DisableOfflineAccessScope bool `json:"disableOfflineAccessScope,omitempty"` } -// CentralsConfig represents the declarative configuration for Central instances defaults and overrides. -type CentralsConfig struct { - AdditionalAuthProviders []AuthProviderAddition `json:"additionalAuthProviders"` -} - // TenantResourceConfig represents the declarative configuration for tenant resource values defaults and overrides. type TenantResourceConfig struct { Default string `json:"default"` @@ -84,18 +76,11 @@ type TenantResourceOverride struct { // ValidateConfig validates the GitOps configuration. func ValidateConfig(config Config) field.ErrorList { var errs field.ErrorList - errs = append(errs, validateCentralsConfig(field.NewPath("centrals"), config.Centrals)...) errs = append(errs, validateTenantResourcesConfig(field.NewPath("tenantResources"), config.TenantResources)...) errs = append(errs, validateApplications(field.NewPath("applications"), config.Applications)...) return errs } -func validateCentralsConfig(path *field.Path, config CentralsConfig) field.ErrorList { - var errs field.ErrorList - errs = append(errs, validateAdditionalAuthProviders(path.Child("additionalAuthProviders"), config.AdditionalAuthProviders)...) - return errs -} - func validateTenantResourcesConfig(path *field.Path, config TenantResourceConfig) field.ErrorList { var errs field.ErrorList errs = append(errs, validateTenantResourcesDefault(path.Child("default"), config.Default)...) @@ -128,132 +113,6 @@ func validateTenantResourceOverride(path *field.Path, override TenantResourceOve return errs } -func validateAdditionalAuthProviders(path *field.Path, providers []AuthProviderAddition) field.ErrorList { - var errs field.ErrorList - for i, additionalProvider := range providers { - errs = append(errs, validateAdditionalAuthProvider(path.Index(i), additionalProvider)...) - } - return errs -} - -func validateAdditionalAuthProvider(path *field.Path, provider AuthProviderAddition) field.ErrorList { - var errs field.ErrorList - - if provider.InstanceID == "" { - errs = append(errs, field.Required(path.Child("instanceId"), "instance ID is required")) - } - authProviderPath := path.Child("authProvider") - errs = append(errs, validateAuthProvider(authProviderPath, provider.AuthProvider)...) - - return errs -} - -func validateAuthProvider(path *field.Path, provider *AuthProvider) field.ErrorList { - var errs field.ErrorList - if provider == nil { - errs = append(errs, field.Required(path, "auth provider spec is required")) - return errs - } - errs = append(errs, validateAuthProviderName(path.Child("name"), provider.Name)...) - errs = append(errs, validateAuthProviderGroups(path.Child("groups"), provider.Groups)...) - errs = append(errs, validateAuthProviderRequiredAttributes(path.Child("requiredAttributes"), provider.RequiredAttributes)...) - errs = append(errs, validateAuthProviderClaimMappings(path.Child("claimMappings"), provider.ClaimMappings)...) - // Empty config means that the config will be copied over from the default auth provider. - if provider.OIDC != nil { - errs = append(errs, validateAuthProviderOIDCConfig(path.Child("oidc"), provider.OIDC)...) - } - return errs -} - -func validateAuthProviderClaimMappings(path *field.Path, claimMappings []AuthProviderClaimMapping) field.ErrorList { - var errs field.ErrorList - for i, claimMapping := range claimMappings { - errs = append(errs, validateAuthProviderClaimMapping(path.Index(i), claimMapping)...) - } - return errs -} - -func validateAuthProviderRequiredAttributes(path *field.Path, requiredAttributes []AuthProviderRequiredAttribute) field.ErrorList { - var errs field.ErrorList - for i, requiredAttribute := range requiredAttributes { - errs = append(errs, validateAuthProviderRequiredAttribute(path.Index(i), requiredAttribute)...) - } - return errs -} - -func validateAuthProviderGroups(path *field.Path, groups []AuthProviderGroup) field.ErrorList { - var errs field.ErrorList - var seenGroups = make(map[AuthProviderGroup]struct{}, len(groups)) - for i, group := range groups { - groupPath := path.Index(i) - if _, ok := seenGroups[group]; ok { - errs = append(errs, field.Duplicate(groupPath, fmt.Sprintf("duplicate group %v", group))) - continue - } - seenGroups[group] = struct{}{} - errs = append(errs, validateAuthProviderGroup(groupPath, group)...) - } - return errs -} - -func validateAuthProviderName(path *field.Path, name string) field.ErrorList { - var errs field.ErrorList - if name == "" { - errs = append(errs, field.Required(path, "name is required")) - } - return errs -} - -func validateAuthProviderOIDCConfig(path *field.Path, config *AuthProviderOIDCConfig) field.ErrorList { - var errs field.ErrorList - if config.ClientID == "" { - errs = append(errs, field.Required(path.Child("clientID"), "clientID is required")) - } - if config.Issuer == "" { - errs = append(errs, field.Required(path.Child("issuer"), "issuer is required")) - } - if config.Mode == "" { - errs = append(errs, field.Required(path.Child("mode"), "callbackMode is required")) - } - return errs -} - -func validateAuthProviderClaimMapping(path *field.Path, claimMapping AuthProviderClaimMapping) field.ErrorList { - var errs field.ErrorList - if claimMapping.Path == "" { - errs = append(errs, field.Required(path.Child("path"), "path is required")) - } - if claimMapping.Name == "" { - errs = append(errs, field.Required(path.Child("name"), "name is required")) - } - return errs -} - -func validateAuthProviderRequiredAttribute(path *field.Path, attribute AuthProviderRequiredAttribute) field.ErrorList { - var errs field.ErrorList - if attribute.Key == "" { - errs = append(errs, field.Required(path.Child("key"), "key is required")) - } - if attribute.Value == "" { - errs = append(errs, field.Required(path.Child("value"), "value is required")) - } - return errs -} - -func validateAuthProviderGroup(path *field.Path, group AuthProviderGroup) field.ErrorList { - var errs field.ErrorList - if group.Role == "" { - errs = append(errs, field.Required(path.Child("role"), "role name is required")) - } - if group.Key == "" { - errs = append(errs, field.Required(path.Child("key"), "key is required")) - } - if group.Value == "" { - errs = append(errs, field.Required(path.Child("value"), "value is required")) - } - return errs -} - // renderDummyValuesWithPatchForValidation renders a dummy tenant resource values with the given patch. // useful to test that a values patch is valid. func renderDummyValuesWithPatchForValidation(patch string) error { diff --git a/internal/central/pkg/gitops/config_test.go b/internal/central/pkg/gitops/config_test.go index bd7276edc0..a803578d85 100644 --- a/internal/central/pkg/gitops/config_test.go +++ b/internal/central/pkg/gitops/config_test.go @@ -3,7 +3,6 @@ package gitops import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/yaml" @@ -30,37 +29,6 @@ rhacsOperators: deploymentName: "stackrox-operator" centralLabelSelector: "app.kubernetes.io/name=central" securedClusterLabelSelector: "app.kubernetes.io/name=securedCluster" -centrals: - additionalAuthProviders: - - instanceId: "id1" - authProvider: - name: "good-name" - groups: - - key: "a" - value: "b" - role: "Admin" - overrides: - - instanceIds: - - id1 - patch: | - {}`, - }, { - name: "invalid auth provider oidc config", - assert: func(t *testing.T, c *Config, err field.ErrorList) { - require.Len(t, err, 3) - }, - yaml: ` -centrals: - additionalAuthProviders: - - instanceId: "id1" - authProvider: - name: "good-name" - oidc: - clientSecret: "donttellanyonemysecret!" - groups: - - key: "a" - value: "b" - role: "Admin" `, }, } @@ -74,183 +42,3 @@ centrals: }) } } - -func TestValidateAdditionalAuthProvider(t *testing.T) { - path := field.NewPath("additionalAuthProvider") - authProviderPath := path.Child("authProvider") - err := validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: nil, - InstanceID: "", - }) - assert.Len(t, err.ToAggregate().Errors(), 2) - assert.Equal(t, field.Required(path.Child("instanceId"), "instance ID is required"), err[0]) - assert.Equal(t, field.Required(authProviderPath, "auth provider spec is required"), err[1]) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: nil, - InstanceID: "non-nil", - }) - require.Len(t, err.ToAggregate().Errors(), 1) - assert.Equal(t, field.Required(authProviderPath, "auth provider spec is required"), err[0]) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{}, - InstanceID: "non-nil", - }) - require.Len(t, err.ToAggregate().Errors(), 1) - assert.Equal(t, field.Required(authProviderPath.Child("name"), "name is required"), err[0]) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - }, - InstanceID: "non-nil", - }) - assert.Nil(t, err) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - OIDC: &AuthProviderOIDCConfig{}, - }, - InstanceID: "non-nil", - }) - require.Len(t, err.ToAggregate().Errors(), 3) - oidcPath := authProviderPath.Child("oidc") - assert.Equal(t, field.Required(oidcPath.Child("clientID"), "clientID is required"), err[0]) - assert.Equal(t, field.Required(oidcPath.Child("issuer"), "issuer is required"), err[1]) - assert.Equal(t, field.Required(oidcPath.Child("mode"), "callbackMode is required"), err[2]) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - OIDC: &AuthProviderOIDCConfig{ - ClientID: "clientID", - Issuer: "issuer", - Mode: "post", - }, - }, - InstanceID: "non-nil", - }) - assert.Nil(t, err) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - OIDC: &AuthProviderOIDCConfig{ - ClientID: "clientID", - Issuer: "issuer", - Mode: "post", - }, - Groups: []AuthProviderGroup{ - {}, - }, - }, - InstanceID: "non-nil", - }) - require.Len(t, err.ToAggregate().Errors(), 3) - groupsPath := authProviderPath.Child("groups") - firstGroupPath := groupsPath.Index(0) - assert.Equal(t, field.Required(firstGroupPath.Child("role"), "role name is required"), err[0]) - assert.Equal(t, field.Required(firstGroupPath.Child("key"), "key is required"), err[1]) - assert.Equal(t, field.Required(firstGroupPath.Child("value"), "value is required"), err[2]) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - OIDC: &AuthProviderOIDCConfig{ - ClientID: "clientID", - Issuer: "issuer", - Mode: "post", - }, - Groups: []AuthProviderGroup{ - { - Role: "role", - Key: "key", - Value: "value", - }, - }, - }, - InstanceID: "non-nil", - }) - assert.Nil(t, err) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - Groups: []AuthProviderGroup{ - { - Role: "role", - Key: "key", - Value: "value", - }, - { - Role: "role", - Key: "key", - Value: "value", - }, - }, - }, - InstanceID: "non-nil", - }) - secondGroupPath := groupsPath.Index(1) - assert.Equal(t, field.Duplicate(secondGroupPath, "duplicate group {key value role}"), err[0]) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - RequiredAttributes: []AuthProviderRequiredAttribute{ - {}, - }, - }, - InstanceID: "non-nil", - }) - require.Len(t, err.ToAggregate().Errors(), 2) - requiredAttributesPath := authProviderPath.Child("requiredAttributes") - firstAttributePath := requiredAttributesPath.Index(0) - assert.Equal(t, field.Required(firstAttributePath.Child("key"), "key is required"), err[0]) - assert.Equal(t, field.Required(firstAttributePath.Child("value"), "value is required"), err[1]) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - RequiredAttributes: []AuthProviderRequiredAttribute{ - { - Key: "key", - Value: "value", - }, - }, - }, - InstanceID: "non-nil", - }) - assert.Nil(t, err) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - ClaimMappings: []AuthProviderClaimMapping{ - {}, - }, - }, - InstanceID: "non-nil", - }) - require.Len(t, err.ToAggregate().Errors(), 2) - claimMappingsPath := authProviderPath.Child("claimMappings") - firstMappingPath := claimMappingsPath.Index(0) - assert.Equal(t, field.Required(firstMappingPath.Child("path"), "path is required"), err[0]) - assert.Equal(t, field.Required(firstMappingPath.Child("name"), "name is required"), err[1]) - - err = validateAdditionalAuthProvider(path, AuthProviderAddition{ - AuthProvider: &AuthProvider{ - Name: "all too well", - ClaimMappings: []AuthProviderClaimMapping{ - { - Path: "path", - Name: "name", - }, - }, - }, - InstanceID: "non-nil", - }) - assert.Nil(t, err) -} diff --git a/internal/central/pkg/presenters/managedcentral.go b/internal/central/pkg/presenters/managedcentral.go index 57ccbdeec1..e14640b0c3 100644 --- a/internal/central/pkg/presenters/managedcentral.go +++ b/internal/central/pkg/presenters/managedcentral.go @@ -104,8 +104,6 @@ func (c *ManagedCentralPresenter) presentManagedCentral(gitopsConfig gitops.Conf if err != nil { return private.ManagedCentral{}, errors.Wrap(err, "failed to get Central YAML") } - authProvider := findAdditionalAuthProvider(gitopsConfig, from) - additionalAuthProvider := constructAdditionalAuthProvider(authProvider, from) res := private.ManagedCentral{ Id: from.ID, Kind: "ManagedCentral", @@ -135,11 +133,10 @@ func (c *ManagedCentralPresenter) presentManagedCentral(gitopsConfig gitops.Conf OwnerAlternateUserId: from.OwnerAlternateUserID, Issuer: from.AuthConfig.Issuer, }, - AdditionalAuthProvider: additionalAuthProvider, - UiHost: from.GetUIHost(), - DataHost: from.GetDataHost(), - TenantResourcesValues: renderedCentral.Values, - InstanceType: from.InstanceType, + UiHost: from.GetUIHost(), + DataHost: from.GetDataHost(), + TenantResourcesValues: renderedCentral.Values, + InstanceType: from.InstanceType, }, RequestStatus: from.Status, } @@ -151,73 +148,6 @@ func (c *ManagedCentralPresenter) presentManagedCentral(gitopsConfig gitops.Conf return res, nil } -func constructAdditionalAuthProvider(authProvider *gitops.AuthProvider, from *dbapi.CentralRequest) private.ManagedCentralAllOfSpecAdditionalAuthProvider { - if authProvider == nil { - return private.ManagedCentralAllOfSpecAdditionalAuthProvider{} - } - oidcConfig := constructAdditionalOidcConfig(authProvider, from) - - groups := make([]private.ManagedCentralAllOfSpecAdditionalAuthProviderGroups, 0, len(authProvider.Groups)) - for _, group := range authProvider.Groups { - groups = append(groups, private.ManagedCentralAllOfSpecAdditionalAuthProviderGroups{ - Key: group.Key, - Value: group.Value, - Role: group.Role, - }) - } - - requiredAttributes := make([]private.ManagedCentralAllOfSpecAdditionalAuthProviderRequiredAttributes, 0, len(authProvider.RequiredAttributes)) - for _, requiredAttribute := range authProvider.RequiredAttributes { - requiredAttributes = append(requiredAttributes, private.ManagedCentralAllOfSpecAdditionalAuthProviderRequiredAttributes{ - Key: requiredAttribute.Key, - Value: requiredAttribute.Value, - }) - } - - claimMappings := make([]private.ManagedCentralAllOfSpecAdditionalAuthProviderRequiredAttributes, 0, len(authProvider.ClaimMappings)) - for _, claimMapping := range authProvider.ClaimMappings { - claimMappings = append(claimMappings, private.ManagedCentralAllOfSpecAdditionalAuthProviderRequiredAttributes{ - Key: claimMapping.Path, - Value: claimMapping.Name, - }) - } - return private.ManagedCentralAllOfSpecAdditionalAuthProvider{ - Name: authProvider.Name, - MinimumRoleName: authProvider.MinimumRole, - Groups: groups, - RequiredAttributes: requiredAttributes, - ClaimMappings: claimMappings, - Oidc: oidcConfig, - } -} - -func constructAdditionalOidcConfig(authProvider *gitops.AuthProvider, from *dbapi.CentralRequest) private.ManagedCentralAllOfSpecAdditionalAuthProviderOidc { - oidcConfig := private.ManagedCentralAllOfSpecAdditionalAuthProviderOidc{} - if authProvider.OIDC != nil && authProvider.OIDC.ClientID != "" { - oidcConfig.ClientID = authProvider.OIDC.ClientID - oidcConfig.ClientSecret = authProvider.OIDC.ClientSecret // pragma: allowlist secret - oidcConfig.Issuer = authProvider.OIDC.Issuer - oidcConfig.CallbackMode = authProvider.OIDC.Mode - oidcConfig.DisableOfflineAccessScope = authProvider.OIDC.DisableOfflineAccessScope - } else { - oidcConfig.ClientID = from.ClientID - oidcConfig.ClientSecret = from.ClientSecret // pragma: allowlist secret - oidcConfig.Issuer = from.Issuer - oidcConfig.CallbackMode = "post" - oidcConfig.DisableOfflineAccessScope = true - } - return oidcConfig -} - -func findAdditionalAuthProvider(gitopsConfig gitops.Config, from *dbapi.CentralRequest) *gitops.AuthProvider { - for _, addition := range gitopsConfig.Centrals.AdditionalAuthProviders { - if addition.InstanceID == from.ID { - return addition.AuthProvider - } - } - return nil -} - func getSecretNames(from *dbapi.CentralRequest) []string { secrets, err := from.Secrets.Object() if err != nil { diff --git a/openapi/fleet-manager-private.yaml b/openapi/fleet-manager-private.yaml index 84f7fe6db1..354ac850d4 100644 --- a/openapi/fleet-manager-private.yaml +++ b/openapi/fleet-manager-private.yaml @@ -229,55 +229,6 @@ components: type: string issuer: type: string - additionalAuthProvider: - type: object - properties: - name: - type: string - minimumRoleName: - type: string - groups: - type: array - items: - type: object - properties: - key: - type: string - value: - type: string - role: - type: string - requiredAttributes: - type: array - items: - type: object - properties: - key: - type: string - value: - type: string - claimMappings: - type: array - items: - type: object - properties: - key: - type: string - value: - type: string - oidc: - type: object - properties: - issuer: - type: string - callbackMode: - type: string - clientID: - type: string - clientSecret: - type: string - disableOfflineAccessScope: - type: boolean uiHost: type: string description: 'Handles GUI/CLI/API connections'