Skip to content

Commit

Permalink
Prevent namespace update in application resource (#4845)
Browse files Browse the repository at this point in the history
# Description

This is the change to prevent namespace update in application resource.
Otherwise, Container resource will be deployed to the different
namespace while having the old pod.

## Issue reference

<!--
We strive to have all PR being opened based on an issue, where the
problem or feature have been discussed prior to implementation.
-->

#3740 

## Checklist

Please make sure you've completed the relevant tasks for this PR, out of
the following list:

* [x] Code compiles correctly
* [x] Adds necessary unit tests for change
* [ ] Adds necessary E2E tests for change
* [x] Unit tests passing
* [ ] Extended the documentation / Created issue for it

Co-authored-by: Aaron Crawfis <[email protected]>
  • Loading branch information
youngbupark and AaronCrawfis authored Dec 9, 2022
1 parent c76ce22 commit 6392f83
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ func (a *CreateOrUpdateApplication) populateKubernetesNamespace(ctx context.Cont
return rest.NewBadRequestResponse(fmt.Sprintf("'%s' is the invalid namespace. This must be at most 63 alphanumeric characters or '-'. Please specify a valid namespace using 'kubernetesNamespace' extension in '$.properties.extensions[*]'.", kubeNamespace)), nil
}

if old != nil {
c := old.Properties.Status.Compute
if c != nil && c.Kind == rp.KubernetesComputeKind && c.KubernetesCompute.Namespace != kubeNamespace {
return rest.NewBadRequestResponse(fmt.Sprintf("Updating an application's Kubernetes namespace from '%s' to '%s' requires the application to be deleted and redeployed. Please delete your application and try again.", c.KubernetesCompute.Namespace, kubeNamespace)), nil
}
}

// Populate kubernetes namespace to internal metadata property for query indexing.
newResource.Properties.Status.Compute = &rp.EnvironmentCompute{
Kind: rp.KubernetesComputeKind,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,22 @@ func TestPopulateKubernetesNamespace_valid_namespace(t *testing.T) {
appCtrl := ctl.(*CreateOrUpdateApplication)

t.Run("override namespace", func(t *testing.T) {
old := &datamodel.Application{
Properties: datamodel.ApplicationProperties{
BasicResourceProperties: rp.BasicResourceProperties{
Environment: testEnvID,
Status: rp.ResourceStatus{
Compute: &rp.EnvironmentCompute{
Kind: rp.KubernetesComputeKind,
KubernetesCompute: rp.KubernetesComputeProperties{
Namespace: "app-ns",
},
},
},
},
},
}

tCtx.MockSC.
EXPECT().
Query(gomock.Any(), gomock.Any()).
Expand Down Expand Up @@ -482,7 +498,7 @@ func TestPopulateKubernetesNamespace_valid_namespace(t *testing.T) {
armctx := &v1.ARMRequestContext{ResourceID: id}
ctx := v1.WithARMRequestContext(tCtx.Ctx, armctx)

resp, err := appCtrl.populateKubernetesNamespace(ctx, nil, newResource)
resp, err := appCtrl.populateKubernetesNamespace(ctx, old, newResource)
require.NoError(t, err)
require.Nil(t, resp)

Expand Down Expand Up @@ -667,4 +683,55 @@ func TestPopulateKubernetesNamespace_invalid_property(t *testing.T) {
res := resp.(*rest.ConflictResponse)
require.Equal(t, res.Body.Error.Message, "Application /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/applications.core/applications/app0 with the same namespace (testns) already exists")
})

t.Run("update application with the different namespace", func(t *testing.T) {
old := &datamodel.Application{
Properties: datamodel.ApplicationProperties{
BasicResourceProperties: rp.BasicResourceProperties{
Environment: testEnvID,
Status: rp.ResourceStatus{
Compute: &rp.EnvironmentCompute{
Kind: rp.KubernetesComputeKind,
KubernetesCompute: rp.KubernetesComputeProperties{
Namespace: "default-app0",
},
},
},
},
},
}

newResource := &datamodel.Application{
BaseResource: v1.BaseResource{
TrackedResource: v1.TrackedResource{
ID: testAppID,
},
},
Properties: datamodel.ApplicationProperties{
BasicResourceProperties: rp.BasicResourceProperties{
Environment: testEnvID,
},
Extensions: []datamodel.Extension{
{
Kind: datamodel.KubernetesNamespaceExtension,
KubernetesNamespace: &datamodel.KubeNamespaceExtension{Namespace: "differentname"},
},
},
},
}

tCtx.MockSC.EXPECT().
Query(gomock.Any(), gomock.Any()).
Return(&store.ObjectQueryResult{}, nil).Times(2)

id, err := resources.ParseResource(testAppID)
require.NoError(t, err)
armctx := &v1.ARMRequestContext{ResourceID: id}
ctx := v1.WithARMRequestContext(tCtx.Ctx, armctx)

resp, err := appCtrl.populateKubernetesNamespace(ctx, old, newResource)
require.NoError(t, err)
res := resp.(*rest.BadRequestResponse)
require.Equal(t, res.Body.Error.Message, "Updating an application's Kubernetes namespace from 'default-app0' to 'differentname' requires the application to be deleted and redeployed. Please delete your application and try again.")
})
}
6 changes: 3 additions & 3 deletions test/functional/corerp/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -594,8 +594,8 @@ func Test_CLI_Only_version(t *testing.T) {
}

func Test_RecipeCommands(t *testing.T) {
template := "testdata/corerp-resources-environment.bicep"
name := "corerp-resources-environment"
template := "testdata/corerp-resources-recipe-env.bicep"
name := "corerp-resources-recipe-env"

requiredSecrets := map[string]map[string]string{}

Expand All @@ -605,7 +605,7 @@ func Test_RecipeCommands(t *testing.T) {
CoreRPResources: &validation.CoreRPResourceSet{
Resources: []validation.CoreRPResource{
{
Name: "corerp-resources-environment-env",
Name: "corerp-resources-recipe-env",
Type: validation.EnvironmentsResource,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import radius as radius
param location string = 'global'

resource env 'Applications.Core/environments@2022-03-15-privatepreview' = {
name: 'corerp-resources-environment-env'
name: 'corerp-resources-recipe-env'
location: location
properties: {
compute: {
kind: 'kubernetes'
resourceId: 'self'
namespace: 'corerp-resources-environment-env'
namespace: 'corerp-resources-recipe-env'
}
recipes: {
recipe1: {
Expand Down

0 comments on commit 6392f83

Please sign in to comment.