Adding changes to extend secret stores scope to global (radius-projec…

…t#7155)

# Description

- Adding changes to support global scope
- Remove appliction as a requred property
- Create namespace provided by user if not exists
- Adding unit tests

Design doc: radius-project/design-notes#38

## Type of change

<!--

Please select **one** of the following options that describes your
change and delete the others. Clearly identifying the type of change you
are making will help us review your PR faster, and is used in authoring
release notes.

If you are making a bug fix or functionality change to Radius and do not
have an associated issue link please create one now.

-->


- This pull request adds or changes features of Radius and has an
approved issue (issue link required).
- This pull request is a minor refactor, code cleanup, test improvement,
or other maintenance task and doesn't change the functionality of Radius
(issue link optional).

<!--

Please update the following to link the associated issue. This is
required for some kinds of changes (see above).

-->

Fixes: radius-project#7030

---------

Signed-off-by: Vishwanath Hiremath <[email protected]>
Signed-off-by: Karishma Chawla <[email protected]>
Co-authored-by: Karishma Chawla <[email protected]>

pkg/corerp/frontend/controller/secretstores/kubernetes.go

@@ -27,6 +27,7 @@ import (
 	"github.com/radius-project/radius/pkg/armrpc/rest"
 	"github.com/radius-project/radius/pkg/corerp/datamodel"
 	"github.com/radius-project/radius/pkg/kubernetes"
+	"github.com/radius-project/radius/pkg/kubeutil"
 	rpv1 "github.com/radius-project/radius/pkg/rp/v1"
 	"github.com/radius-project/radius/pkg/to"
 	"github.com/radius-project/radius/pkg/ucp/resources"
@@ -184,6 +185,11 @@ func UpsertSecret(ctx context.Context, newResource, old *datamodel.SecretStore,
 		ref = old.Properties.Resource
 	}
 
+	// resource property cannot be empty for global scoped resource.
+	if newResource.Properties.BasicResourceProperties.IsGlobalScopedResource() && ref == "" {
+		return rest.NewBadRequestResponse("$.properties.resource cannot be empty for global scoped resource."), nil
+	}
+
 	ns, name, err := fromResourceID(ref)
 	if err != nil {
 		return nil, err
@@ -195,6 +201,12 @@ func UpsertSecret(ctx context.Context, newResource, old *datamodel.SecretStore,
 		}
 	}
 
+	// Create namespace if not exists.
+	err = kubeutil.PatchNamespace(ctx, options.KubeClient, ns)
+	if err != nil {
+		return nil, err
+	}
+
 	if name == "" {
 		name = newResource.Name
 	}
@@ -208,8 +220,9 @@ func UpsertSecret(ctx context.Context, newResource, old *datamodel.SecretStore,
 	ksecret := &corev1.Secret{}
 	err = options.KubeClient.Get(ctx, runtimeclient.ObjectKey{Namespace: ns, Name: name}, ksecret)
 	if apierrors.IsNotFound(err) {
-		// If resource in incoming request references resource, then the resource must exist.
-		if ref != "" {
+		// If resource in incoming request references resource, then the resource must exist for a application/environment scoped resource.
+		// For global scoped resource create the kubernetes resource if not exists.
+		if ref != "" && !newResource.Properties.BasicResourceProperties.IsGlobalScopedResource() {
 			return rest.NewBadRequestResponse(fmt.Sprintf("'%s' referenced resource does not exist.", ref)), nil
 		}
 		app, _ := resources.ParseResource(newResource.Properties.Application)

pkg/corerp/frontend/controller/secretstores/kubernetes_test.go

@@ -45,9 +45,12 @@ const (
 	testEnvID     = testRootScope + "/Applications.Core/environments/env0"
 	testAppID     = testRootScope + "/Applications.Core/applications/app0"
 
-	testFileCertValueFrom = "secretstores_datamodel_cert_valuefrom.json"
-	testFileCertValue     = "secretstores_datamodel_cert_value.json"
-	testFileGenericValue  = "secretstores_datamodel_generic.json"
+	testFileCertValueFrom               = "secretstores_datamodel_cert_valuefrom.json"
+	testFileCertValue                   = "secretstores_datamodel_cert_value.json"
+	testFileGenericValue                = "secretstores_datamodel_generic.json"
+	testFileGenericValueGlobalScope     = "secretstores_datamodel_global_scope.json"
+	testFileGenericValueInvalidResource = "secretstores_datamodel_global_scope_invalid_resource.json"
+	testFileGenericValueEmptyResource   = "secretstores_datamodel_global_scope_empty_resource.json"
 )
 
 func TestGetNamespace(t *testing.T) {
@@ -466,6 +469,109 @@ func TestUpsertSecret(t *testing.T) {
 		require.Equal(t, "'app0-ns/secret1' of $.properties.resource must be same as 'app0-ns/secret0'.", r.Body.Error.Message)
 	})
 
+	t.Run("create a new secret resource with global scope", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		sc := store.NewMockStorageClient(ctrl)
+
+		newResource := testutil.MustGetTestData[datamodel.SecretStore](testFileGenericValueGlobalScope)
+
+		opt := &controller.Options{
+			StorageClient: sc,
+			KubeClient:    k8sutil.NewFakeKubeClient(nil),
+		}
+
+		_, err := ValidateAndMutateRequest(context.TODO(), newResource, nil, opt)
+		require.NoError(t, err)
+		_, err = UpsertSecret(context.TODO(), newResource, nil, opt)
+		require.NoError(t, err)
+
+		// assert
+		require.Equal(t, "test-namespace/secret0", newResource.Properties.Resource)
+		ksecret := &corev1.Secret{}
+
+		err = opt.KubeClient.Get(context.TODO(), runtimeclient.ObjectKey{Namespace: "test-namespace", Name: "secret0"}, ksecret)
+		require.NoError(t, err)
+
+		require.Equal(t, "dGxzLmNydA==", string(ksecret.Data["tls.crt"]))
+		require.Equal(t, "dGxzLmNlcnQK", string(ksecret.Data["tls.key"]))
+		require.Equal(t, "MTAwMDAwMDAtMTAwMC0xMDAwLTAwMDAtMDAwMDAwMDAwMDAw", string(ksecret.Data["servicePrincipalPassword"]))
+		require.Equal(t, rpv1.OutputResource{
+			LocalID: "Secret",
+			ID: resources_kubernetes.IDFromParts(
+				resources_kubernetes.PlaneNameTODO,
+				"",
+				resources_kubernetes.KindSecret,
+				"test-namespace",
+				"secret0"),
+		}, newResource.Properties.Status.OutputResources[0])
+	})
+
+	t.Run("create a new secret resource with invalid resource", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		sc := store.NewMockStorageClient(ctrl)
+
+		newResource := testutil.MustGetTestData[datamodel.SecretStore](testFileGenericValueInvalidResource)
+
+		opt := &controller.Options{
+			StorageClient: sc,
+			KubeClient:    k8sutil.NewFakeKubeClient(nil),
+		}
+
+		_, err := ValidateAndMutateRequest(context.TODO(), newResource, nil, opt)
+		require.NoError(t, err)
+		_, err = UpsertSecret(context.TODO(), newResource, nil, opt)
+		require.Error(t, err)
+		require.Equal(t, err.Error(), "no Kubernetes namespace")
+	})
+
+	t.Run("create a new secret resource with empty resource", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		sc := store.NewMockStorageClient(ctrl)
+
+		newResource := testutil.MustGetTestData[datamodel.SecretStore](testFileGenericValueEmptyResource)
+
+		opt := &controller.Options{
+			StorageClient: sc,
+			KubeClient:    k8sutil.NewFakeKubeClient(nil),
+		}
+
+		_, err := ValidateAndMutateRequest(context.TODO(), newResource, nil, opt)
+		require.NoError(t, err)
+		resp, err := UpsertSecret(context.TODO(), newResource, nil, opt)
+		require.NoError(t, err)
+
+		// assert
+		r := resp.(*rest.BadRequestResponse)
+		require.Equal(t, "$.properties.resource cannot be empty for global scoped resource.", r.Body.Error.Message)
+	})
+
+	t.Run("add secret values to the existing secret store 1 ", func(t *testing.T) {
+		newResource := testutil.MustGetTestData[datamodel.SecretStore](testFileCertValue)
+		newResource.Properties.Resource = "default/secret"
+
+		opt := &controller.Options{
+			KubeClient: k8sutil.NewFakeKubeClient(nil),
+		}
+
+		resp, _ := UpsertSecret(context.TODO(), newResource, nil, opt)
+		r := resp.(*rest.BadRequestResponse)
+		require.Equal(t, "'default/secret' referenced resource does not exist.", r.Body.Error.Message)
+	})
+
+	t.Run("inherit old resource id for global scoped resource", func(t *testing.T) {
+		oldResource := testutil.MustGetTestData[datamodel.SecretStore](testFileGenericValueGlobalScope)
+		newResource := testutil.MustGetTestData[datamodel.SecretStore](testFileGenericValueEmptyResource)
+
+		opt := &controller.Options{
+			KubeClient: k8sutil.NewFakeKubeClient(nil),
+		}
+
+		_, err := UpsertSecret(context.TODO(), newResource, oldResource, opt)
+		require.NoError(t, err)
+
+		// assert
+		require.Equal(t, oldResource.Properties.Resource, newResource.Properties.Resource)
+	})
 }
 
 func TestDeleteSecret(t *testing.T) {

pkg/corerp/frontend/controller/secretstores/testdata/secretstores_datamodel_global_scope.json

@@ -0,0 +1,37 @@
+{
+    "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": {
+      "resource": "test-namespace/secret0",
+      "type": "generic",
+      "data": {
+        "tls.crt": {
+          "encoding": "raw",
+          "value": "tls.crt"
+        },
+        "tls.key": {
+          "encoding": "base64",
+          "value": "dGxzLmNlcnQK"
+        },
+        "servicePrincipalPassword": {
+          "value": "10000000-1000-1000-0000-000000000000"
+        }
+      }
+    },
+    "tenantId": "00000000-0000-0000-0000-000000000000",
+    "subscriptionId": "00000000-0000-0000-0000-000000000000",
+    "resourceGroup": "testGroup",
+    "createdApiVersion": "2023-10-01-preview",
+    "updatedApiVersion": "2023-10-01-preview"
+  }

pkg/corerp/frontend/controller/secretstores/testdata/secretstores_datamodel_global_scope_empty_resource.json

@@ -0,0 +1,36 @@
+{
+    "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": {
+      "type": "generic",
+      "data": {
+        "tls.crt": {
+          "encoding": "raw",
+          "value": "tls.crt"
+        },
+        "tls.key": {
+          "encoding": "base64",
+          "value": "dGxzLmNlcnQK"
+        },
+        "servicePrincipalPassword": {
+          "value": "10000000-1000-1000-0000-000000000000"
+        }
+      }
+    },
+    "tenantId": "00000000-0000-0000-0000-000000000000",
+    "subscriptionId": "00000000-0000-0000-0000-000000000000",
+    "resourceGroup": "testGroup",
+    "createdApiVersion": "2023-10-01-preview",
+    "updatedApiVersion": "2023-10-01-preview"
+  }

pkg/corerp/frontend/controller/secretstores/testdata/secretstores_datamodel_global_scope_invalid_resource.json

@@ -0,0 +1,37 @@
+{
+    "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": {
+      "resource": "secret0",
+      "type": "generic",
+      "data": {
+        "tls.crt": {
+          "encoding": "raw",
+          "value": "tls.crt"
+        },
+        "tls.key": {
+          "encoding": "base64",
+          "value": "dGxzLmNlcnQK"
+        },
+        "servicePrincipalPassword": {
+          "value": "10000000-1000-1000-0000-000000000000"
+        }
+      }
+    },
+    "tenantId": "00000000-0000-0000-0000-000000000000",
+    "subscriptionId": "00000000-0000-0000-0000-000000000000",
+    "resourceGroup": "testGroup",
+    "createdApiVersion": "2023-10-01-preview",
+    "updatedApiVersion": "2023-10-01-preview"
+  }

pkg/rp/v1/types.go

@@ -55,6 +55,11 @@ func (b *BasicResourceProperties) EqualLinkedResource(prop *BasicResourcePropert
 	return strings.EqualFold(b.Application, prop.Application) && strings.EqualFold(b.Environment, prop.Environment)
 }
 
+// Method IsGlobalScopedResource checks if resource is global scoped.
+func (b *BasicResourceProperties) IsGlobalScopedResource() bool {
+	return b.Application == "" && b.Environment == ""
+}
+
 // ResourceStatus represents the output status of Radius resource.
 type ResourceStatus struct {
 	// Compute represents a resource presented in the underlying platform.

pkg/rp/v1/types_test.go

@@ -96,3 +96,38 @@ func TestEqualLinkedResource(t *testing.T) {
 		require.Equal(t, tt.propA.EqualLinkedResource(&tt.propB), tt.eq)
 	}
 }
+
+func Test_isGlobalScopedResource(t *testing.T) {
+	tests := []struct {
+		desc                    string
+		basicResourceProperties BasicResourceProperties
+		isGlobal                bool
+	}{
+		{
+			desc: "application scope",
+			basicResourceProperties: BasicResourceProperties{
+				Application: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/Applications.Core/applications/app1",
+			},
+			isGlobal: false,
+		},
+		{
+			desc: "environment scope",
+			basicResourceProperties: BasicResourceProperties{
+				Environment: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/Applications.core/environments/env0",
+			},
+			isGlobal: false,
+		},
+		{
+			desc:                    "global scope",
+			basicResourceProperties: BasicResourceProperties{},
+			isGlobal:                true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			act := tt.basicResourceProperties.IsGlobalScopedResource()
+			require.Equal(t, act, tt.isGlobal)
+		})
+	}
+
+}