Skip to content

Commit

Permalink
Adding changes to extend secret stores scope to global (#7155)
Browse files Browse the repository at this point in the history
# 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: #7030

---------

Signed-off-by: Vishwanath Hiremath <[email protected]>
Signed-off-by: Karishma Chawla <[email protected]>
Co-authored-by: Karishma Chawla <[email protected]>
  • Loading branch information
vishwahiremat and kachawla authored Feb 23, 2024
1 parent 275b861 commit 6885574
Show file tree
Hide file tree
Showing 14 changed files with 397 additions and 11 deletions.

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions pkg/corerp/api/v20231001preview/zz_generated_models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 15 additions & 2 deletions pkg/corerp/frontend/controller/secretstores/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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)
Expand Down
112 changes: 109 additions & 3 deletions pkg/corerp/frontend/controller/secretstores/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
@@ -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"
}
5 changes: 5 additions & 0 deletions pkg/rp/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
35 changes: 35 additions & 0 deletions pkg/rp/v1/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

}
Loading

0 comments on commit 6885574

Please sign in to comment.