Add new secret types to Applications.Core/secretstores (radius-projec…

…t#7816) # Description Add new types to Applications.Core/secretstores (basicAuthentication, azureWorkloadIdentity, awsIRSA) Update convertor, tests. Update existing ValidateAndMutateRequest() in /pkg/corerp/frontend/controller/secretstores/kubernetes.go to check if required secret keys exist for current secret type. Add to existing unit tests. ## Type of change - This pull request fixes a bug in Radius and has an approved issue (radius-project#6917 ). Fixes: Part of radius-project#6917
lakshmimsft · Aug 29, 2024 · 50fef93 · 50fef93
1 parent 84cb120
commit 50fef93
Show file tree

Hide file tree

Showing 16 changed files with 412 additions and 104 deletions.
diff --git a/...bicep-types-radius/generated/applications/applications.core/2023-10-01-preview/types.json b/...bicep-types-radius/generated/applications/applications.core/2023-10-01-preview/types.json
diff --git a/hack/bicep-types-radius/generated/index.json b/hack/bicep-types-radius/generated/index.json
@@ -16,10 +16,10 @@
       "$ref": "applications/applications.core/2023-10-01-preview/types.json#/197"
     },
     "Applications.Core/secretStores@2023-10-01-preview": {
-      "$ref": "applications/applications.core/2023-10-01-preview/types.json#/226"
+      "$ref": "applications/applications.core/2023-10-01-preview/types.json#/232"
     },
     "Applications.Core/volumes@2023-10-01-preview": {
-      "$ref": "applications/applications.core/2023-10-01-preview/types.json#/263"
+      "$ref": "applications/applications.core/2023-10-01-preview/types.json#/269"
     },
     "Applications.Dapr/pubSubBrokers@2023-10-01-preview": {
       "$ref": "applications/applications.dapr/2023-10-01-preview/types.json#/44"

diff --git a/pkg/corerp/api/v20231001preview/secretstore_conversion.go b/pkg/corerp/api/v20231001preview/secretstore_conversion.go
@@ -106,6 +106,12 @@ func toSecretStoreDataTypeDataModel(src *SecretStoreDataType) datamodel.SecretTy
 		return datamodel.SecretTypeGeneric
 	case SecretStoreDataTypeCertificate:
 		return datamodel.SecretTypeCert
+	case SecretStoreDataTypeBasicAuthentication:
+		return datamodel.SecretTypeBasicAuthentication
+	case SecretStoreDataTypeAzureWorkloadIdentity:
+		return datamodel.SecretTypeAzureWorkloadIdentity
+	case SecretStoreDataTypeAwsIRSA:
+		return datamodel.SecretTypeAWSIRSA
 	}
 
 	return datamodel.SecretTypeGeneric
@@ -117,6 +123,12 @@ func fromSecretStoreDataTypeDataModel(src datamodel.SecretType) *SecretStoreData
 		return to.Ptr(SecretStoreDataTypeGeneric)
 	case datamodel.SecretTypeCert:
 		return to.Ptr(SecretStoreDataTypeCertificate)
+	case datamodel.SecretTypeBasicAuthentication:
+		return to.Ptr(SecretStoreDataTypeBasicAuthentication)
+	case datamodel.SecretTypeAzureWorkloadIdentity:
+		return to.Ptr(SecretStoreDataTypeAzureWorkloadIdentity)
+	case datamodel.SecretTypeAWSIRSA:
+		return to.Ptr(SecretStoreDataTypeAwsIRSA)
 	}
 	return nil
 }

diff --git a/pkg/corerp/api/v20231001preview/zz_generated_constants.go b/pkg/corerp/api/v20231001preview/zz_generated_constants.go
diff --git a/pkg/corerp/api/v20231001preview/zz_generated_models.go b/pkg/corerp/api/v20231001preview/zz_generated_models.go
diff --git a/pkg/corerp/datamodel/secretstore.go b/pkg/corerp/datamodel/secretstore.go
@@ -45,6 +45,12 @@ const (
 	SecretTypeGeneric SecretType = "generic"
 	// SecretTypeCert is the certificate secret type.
 	SecretTypeCert SecretType = "certificate"
+	// SecretTypeBasicAuthentication is the basicAuthentication secret type.
+	SecretTypeBasicAuthentication SecretType = "basicAuthentication"
+	// SecretTypeAzureWorkloadIdentity is the azureWorkloadIdentity secret type.
+	SecretTypeAzureWorkloadIdentity SecretType = "azureWorkloadIdentity"
+	// SecretTypeAWSIRSA is the awsIRSA secret type.
+	SecretTypeAWSIRSA SecretType = "awsIRSA"
 )
 
 // SecretStore represents secret store resource.

diff --git a/pkg/corerp/frontend/controller/secretstores/kubernetes.go b/pkg/corerp/frontend/controller/secretstores/kubernetes.go
@@ -46,6 +46,9 @@ func getOrDefaultType(t datamodel.SecretType) (datamodel.SecretType, error) {
 		t = datamodel.SecretTypeGeneric
 	case datamodel.SecretTypeCert:
 	case datamodel.SecretTypeGeneric:
+	case datamodel.SecretTypeBasicAuthentication:
+	case datamodel.SecretTypeAzureWorkloadIdentity:
+	case datamodel.SecretTypeAWSIRSA:
 	default:
 		err = fmt.Errorf("'%s' is invalid secret type", t)
 	}
@@ -75,8 +78,15 @@ func getOrDefaultEncoding(t datamodel.SecretType, e datamodel.SecretValueEncodin
 	return e, err
 }
 
+// Define a map of required keys for each SecretType
+var requiredKeys = map[datamodel.SecretType][]string{
+	datamodel.SecretTypeBasicAuthentication:   {RequiredUsername, RequiredPassword},
+	datamodel.SecretTypeAzureWorkloadIdentity: {RequiredClientId, RequiredTenantId},
+	datamodel.SecretTypeAWSIRSA:               {RequiredRoleARN},
+}
+
 // ValidateAndMutateRequest checks the type and encoding of the secret store, and ensures that the secret store data is
-// valid. If any of these checks fail, a BadRequestResponse is returned.
+// valid and required keys are present for the secret type. If any of these checks fail, a BadRequestResponse is returned.
 func ValidateAndMutateRequest(ctx context.Context, newResource *datamodel.SecretStore, oldResource *datamodel.SecretStore, options *controller.Options) (rest.Response, error) {
 	var err error
 	newResource.Properties.Type, err = getOrDefaultType(newResource.Properties.Type)
@@ -116,6 +126,15 @@ func ValidateAndMutateRequest(ctx context.Context, newResource *datamodel.Secret
 		}
 	}
 
+	// Validate that required keys for the secret type are present in the secret data
+	if keys, ok := requiredKeys[newResource.Properties.Type]; ok {
+		for _, key := range keys {
+			if _, ok := newResource.Properties.Data[key]; !ok {
+				return rest.NewBadRequestResponse(fmt.Sprintf("$.properties.data must contain '%s' key for %s type.", key, newResource.Properties.Type)), nil
+			}
+		}
+	}
+
 	return nil, nil
 }
 

diff --git a/pkg/corerp/frontend/controller/secretstores/kubernetes_test.go b/pkg/corerp/frontend/controller/secretstores/kubernetes_test.go
@@ -51,6 +51,11 @@ const (
 	testFileGenericValueGlobalScope     = "secretstores_datamodel_global_scope.json"
 	testFileGenericValueInvalidResource = "secretstores_datamodel_global_scope_invalid_resource.json"
 	testFileGenericValueEmptyResource   = "secretstores_datamodel_global_scope_empty_resource.json"
+
+	testFileBasicAuthentication        = "secretstores_datamodel_basicauth.json"
+	testFileBasicAuthenticationInvalid = "secretstores_datamodel_basicauth_invalid.json"
+	testFileAWSIRSA                    = "secretstores_datamodel_awsirsa.json"
+	testFileAzureWorkloadIdentity      = "secretstores_datamodel_azwi.json"
 )
 
 func TestGetNamespace(t *testing.T) {
@@ -247,53 +252,109 @@ func TestGetOrDefaultEncoding(t *testing.T) {
 }
 
 func TestValidateAndMutateRequest(t *testing.T) {
-	t.Run("default type is generic", func(t *testing.T) {
-		newResource := testutil.MustGetTestData[datamodel.SecretStore](testFileCertValueFrom)
-		newResource.Properties.Type = ""
-
-		resp, err := ValidateAndMutateRequest(context.TODO(), newResource, nil, nil)
-		require.NoError(t, err)
-		require.Nil(t, resp)
-
-		// assert
-		require.Equal(t, datamodel.SecretTypeGeneric, newResource.Properties.Type)
-	})
-
-	t.Run("new resource, but referencing valueFrom", func(t *testing.T) {
-		newResource := testutil.MustGetTestData[datamodel.SecretStore](testFileCertValueFrom)
-		newResource.Properties.Resource = ""
-		resp, err := ValidateAndMutateRequest(context.TODO(), newResource, nil, nil)
-		require.NoError(t, err)
-
-		// assert
-		r := resp.(*rest.BadRequestResponse)
-		require.True(t, r.Body.Error.Message == "$.properties.data[tls.crt].Value must be given to create the secret." ||
-			r.Body.Error.Message == "$.properties.data[tls.key].Value must be given to create the secret.")
-	})
-
-	t.Run("update the existing resource - type not matched", func(t *testing.T) {
-		oldResource := testutil.MustGetTestData[datamodel.SecretStore](testFileCertValueFrom)
-		oldResource.Properties.Type = datamodel.SecretTypeGeneric
-		newResource := testutil.MustGetTestData[datamodel.SecretStore](testFileCertValueFrom)
-		resp, err := ValidateAndMutateRequest(context.TODO(), newResource, oldResource, nil)
-		require.NoError(t, err)
-
-		// assert
-		r := resp.(*rest.BadRequestResponse)
-		require.Equal(t, "$.properties.type cannot change from 'generic' to 'certificate'.", r.Body.Error.Message)
-	})
-
-	t.Run("inherit resource id from existing resource", func(t *testing.T) {
-		oldResource := testutil.MustGetTestData[datamodel.SecretStore](testFileCertValueFrom)
-		newResource := testutil.MustGetTestData[datamodel.SecretStore](testFileCertValueFrom)
-		newResource.Properties.Resource = ""
-		resp, err := ValidateAndMutateRequest(context.TODO(), newResource, oldResource, nil)
+	tests := []struct {
+		name           string
+		testFile       string
+		oldResource    *datamodel.SecretStore
+		modifyResource func(*datamodel.SecretStore, *datamodel.SecretStore)
+		assertions     func(*testing.T, rest.Response, error, *datamodel.SecretStore, *datamodel.SecretStore)
+	}{
+		{
+			name:     "default type is generic",
+			testFile: testFileCertValueFrom,
+			modifyResource: func(newResource, oldResource *datamodel.SecretStore) {
+				newResource.Properties.Type = ""
+			},
+			assertions: func(t *testing.T, resp rest.Response, err error, newResource, oldResource *datamodel.SecretStore) {
+				require.NoError(t, err)
+				require.Nil(t, resp)
+				require.Equal(t, datamodel.SecretTypeGeneric, newResource.Properties.Type)
+			},
+		},
+		{
+			name:     "new resource, but referencing valueFrom",
+			testFile: testFileCertValueFrom,
+			modifyResource: func(newResource, oldResource *datamodel.SecretStore) {
+				newResource.Properties.Resource = ""
+			},
+			assertions: func(t *testing.T, resp rest.Response, err error, newResource, oldResource *datamodel.SecretStore) {
+				require.NoError(t, err)
+				r := resp.(*rest.BadRequestResponse)
+				require.True(t, r.Body.Error.Message == "$.properties.data[tls.crt].Value must be given to create the secret." ||
+					r.Body.Error.Message == "$.properties.data[tls.key].Value must be given to create the secret.")
+			},
+		},
+		{
+			name:        "update the existing resource - type not matched",
+			testFile:    testFileCertValueFrom,
+			oldResource: testutil.MustGetTestData[datamodel.SecretStore](testFileCertValueFrom),
+			modifyResource: func(newResource, oldResource *datamodel.SecretStore) {
+				oldResource.Properties.Type = datamodel.SecretTypeGeneric
+			},
+			assertions: func(t *testing.T, resp rest.Response, err error, newResource, oldResource *datamodel.SecretStore) {
+				require.NoError(t, err)
+				r := resp.(*rest.BadRequestResponse)
+				require.Equal(t, "$.properties.type cannot change from 'generic' to 'certificate'.", r.Body.Error.Message)
+			},
+		},
+		{
+			name:        "inherit resource id from existing resource",
+			testFile:    testFileCertValueFrom,
+			oldResource: testutil.MustGetTestData[datamodel.SecretStore](testFileCertValueFrom),
+			modifyResource: func(newResource, oldResource *datamodel.SecretStore) {
+				newResource.Properties.Resource = ""
+			},
+			assertions: func(t *testing.T, resp rest.Response, err error, newResource, oldResource *datamodel.SecretStore) {
+				require.NoError(t, err)
+				require.Nil(t, resp)
+				require.Equal(t, oldResource.Properties.Resource, newResource.Properties.Resource)
+			},
+		},
+		{
+			name:     "new basicAuthentication resource",
+			testFile: testFileBasicAuthentication,
+			assertions: func(t *testing.T, resp rest.Response, err error, newResource, oldResource *datamodel.SecretStore) {
+				require.NoError(t, err)
+				require.Nil(t, resp)
+			},
+		},
+		{
+			name:     "new awsIRSA resource",
+			testFile: testFileAWSIRSA,
+			assertions: func(t *testing.T, resp rest.Response, err error, newResource, oldResource *datamodel.SecretStore) {
+				require.NoError(t, err)
+				require.Nil(t, resp)
+			},
+		},
+		{
+			name:     "new azureWorkloadIdentity resource",
+			testFile: testFileAzureWorkloadIdentity,
+			assertions: func(t *testing.T, resp rest.Response, err error, newResource, oldResource *datamodel.SecretStore) {
+				require.NoError(t, err)
+				require.Nil(t, resp)
+			},
+		},
+		{
+			name:     "invalid basicAuthentication resource",
+			testFile: testFileBasicAuthenticationInvalid,
+			assertions: func(t *testing.T, resp rest.Response, err error, newResource, oldResource *datamodel.SecretStore) {
+				require.NoError(t, err)
+				r := resp.(*rest.BadRequestResponse)
+				require.True(t, r.Body.Error.Message == "$.properties.data must contain 'password' key for basicAuthentication type.")
+			},
+		},
+	}
 
-		// assert
-		require.NoError(t, err)
-		require.Nil(t, resp)
-		require.Equal(t, oldResource.Properties.Resource, newResource.Properties.Resource)
-	})
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			newResource := testutil.MustGetTestData[datamodel.SecretStore](tt.testFile)
+			if tt.modifyResource != nil {
+				tt.modifyResource(newResource, tt.oldResource)
+			}
+			resp, err := ValidateAndMutateRequest(context.TODO(), newResource, tt.oldResource, nil)
+			tt.assertions(t, resp, err, newResource, tt.oldResource)
+		})
+	}
 }
 
 func TestUpsertSecret(t *testing.T) {

diff --git a/pkg/corerp/frontend/controller/secretstores/testdata/secretstores_datamodel_awsirsa.json b/pkg/corerp/frontend/controller/secretstores/testdata/secretstores_datamodel_awsirsa.json
@@ -0,0 +1,29 @@
+{
+  "id": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/secretStores/secret0",
+  "name": "secret0",
+  "type": "applications.core/secretstores",
+  "location": "global",
+  "systemData": {
+    "createdAt": "2022-03-22T18:54:52.6857175Z",
+    "createdBy": "[email protected]",
+    "createdByType": "User",
+    "lastModifiedAt": "2022-03-22T18:57:52.6857175Z",
+    "lastModifiedBy": "[email protected]",
+    "lastModifiedByType": "User"
+  },
+  "provisioningState": "Succeeded",
+  "properties": {
+    "application": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/applications/app0",
+    "type": "awsIRSA",
+    "data": {
+      "roleARN": {
+        "value": "test-role-arn"
+      }
+    }
+  },
+  "tenantId": "00000000-0000-0000-0000-000000000000",
+  "subscriptionId": "00000000-0000-0000-0000-000000000000",
+  "resourceGroup": "testGroup",
+  "createdApiVersion": "2023-10-01-preview",
+  "updatedApiVersion": "2023-10-01-preview"
+}
diff --git a/pkg/corerp/frontend/controller/secretstores/testdata/secretstores_datamodel_azwi.json b/pkg/corerp/frontend/controller/secretstores/testdata/secretstores_datamodel_azwi.json
@@ -0,0 +1,32 @@
+{
+  "id": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/secretStores/secret0",
+  "name": "secret0",
+  "type": "applications.core/secretstores",
+  "location": "global",
+  "systemData": {
+    "createdAt": "2022-03-22T18:54:52.6857175Z",
+    "createdBy": "[email protected]",
+    "createdByType": "User",
+    "lastModifiedAt": "2022-03-22T18:57:52.6857175Z",
+    "lastModifiedBy": "[email protected]",
+    "lastModifiedByType": "User"
+  },
+  "provisioningState": "Succeeded",
+  "properties": {
+    "application": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/applications/app0",
+    "type": "azureWorkloadIdentity",
+    "data": {
+      "clientId": {
+        "value": "test-client-Id"
+      },
+      "tenantId": {
+        "value": "test-tenant-Id"
+      }
+    }
+  },
+  "tenantId": "00000000-0000-0000-0000-000000000000",
+  "subscriptionId": "00000000-0000-0000-0000-000000000000",
+  "resourceGroup": "testGroup",
+  "createdApiVersion": "2023-10-01-preview",
+  "updatedApiVersion": "2023-10-01-preview"
+}
diff --git a/pkg/corerp/frontend/controller/secretstores/testdata/secretstores_datamodel_basicauth.json b/pkg/corerp/frontend/controller/secretstores/testdata/secretstores_datamodel_basicauth.json
@@ -0,0 +1,32 @@
+{
+  "id": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/secretStores/secret0",
+  "name": "secret0",
+  "type": "applications.core/secretstores",
+  "location": "global",
+  "systemData": {
+    "createdAt": "2022-03-22T18:54:52.6857175Z",
+    "createdBy": "[email protected]",
+    "createdByType": "User",
+    "lastModifiedAt": "2022-03-22T18:57:52.6857175Z",
+    "lastModifiedBy": "[email protected]",
+    "lastModifiedByType": "User"
+  },
+  "provisioningState": "Succeeded",
+  "properties": {
+    "application": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/applications/app0",
+    "type": "basicAuthentication",
+    "data": {
+      "username": {
+        "value": "uname123"
+      },
+      "password": {
+        "value": "testpwd-dGxzLmNlcnQK"
+      }
+    }
+  },
+  "tenantId": "00000000-0000-0000-0000-000000000000",
+  "subscriptionId": "00000000-0000-0000-0000-000000000000",
+  "resourceGroup": "testGroup",
+  "createdApiVersion": "2023-10-01-preview",
+  "updatedApiVersion": "2023-10-01-preview"
+}