Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding changes to extend secret stores scope to global #7155

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

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,
"",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this make a secret resource global scoped?

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",
kachawla marked this conversation as resolved.
Show resolved Hide resolved
"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
Loading