Skip to content

Commit

Permalink
Fix 500 panic when generated Kubernetes namespace is too long (#6349)
Browse files Browse the repository at this point in the history
# Description

Adding logic to CreateAppScopedNamespace to check if the generated name
is too long

## 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 fixes a bug in Radius and has an approved issue
(issue link required).

<!--

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

-->

Fixes: #6292

## Auto-generated summary

<!--
GitHub Copilot for docs will auto-generate a summary of the PR
-->

<!--
copilot:all
-->
### <samp>🤖 Generated by Copilot at 71bf2ae</samp>

### Summary
🧪🔒🚀

<!--
1. 🧪 - This emoji represents testing, and it can be used for the first
change that adds a unit test case for the validation check.
2. 🔒 - This emoji represents security or validation, and it can be used
for the second change that adds a validation check for the generated
namespace name.
3. 🚀 - This emoji represents deployment or launching, and it can be used
for the overall feature of creating application scoped namespaces.
-->
Add validation and testing for application scoped namespace names.
Ensure that the namespace name derived from the environment and
application names in `updatefilter.go` is valid for Kubernetes, and add
a unit test case in `updatefilter_test.go` to verify the behavior.

> _`CreateAppScopedNamespace`_
> _Validates namespace name - spring_
> _Bad request if too long_

### Walkthrough
* Validate namespace name for application scoped to environment
([link](https://github.com/radius-project/radius/pull/6349/files?diff=unified&w=0#diff-0ac671e5a672d80935544b559d7bed83e9456c8895f594b400bc86111e1ca9d6L71-R78))
* Test validation check for namespace name
([link](https://github.com/radius-project/radius/pull/6349/files?diff=unified&w=0#diff-9d9a183263ff9a4aecc0001864dfb4346a16375fdc7263c2e3295fa30cfb793dR162-R200))
  • Loading branch information
willdavsmith authored Sep 27, 2023
1 parent 86ca6d8 commit 2ca32a6
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
9 changes: 8 additions & 1 deletion pkg/corerp/frontend/controller/applications/updatefilter.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,14 @@ func CreateAppScopedNamespace(ctx context.Context, newResource, oldResource *dat
return rest.NewBadRequestResponse(fmt.Sprintf("Environment %s could not be constructed: %s",
newResource.Properties.Environment, err.Error())), nil
}
kubeNamespace = kubernetes.NormalizeResourceName(fmt.Sprintf("%s-%s", envNamespace, serviceCtx.ResourceID.Name()))

namespace := fmt.Sprintf("%s-%s", envNamespace, serviceCtx.ResourceID.Name())
if !kubernetes.IsValidObjectName(namespace) {
return rest.NewBadRequestResponse(fmt.Sprintf("Application namespace '%s' could not be created: the combination of application and environment names is too long.",
namespace)), nil
}

kubeNamespace = kubernetes.NormalizeResourceName(namespace)
}

// Check if another environment resource is using namespace
Expand Down
39 changes: 39 additions & 0 deletions pkg/corerp/frontend/controller/applications/updatefilter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,45 @@ func TestCreateAppScopedNamespace_invalid_property(t *testing.T) {
KubeClient: k8sutil.NewFakeKubeClient(nil),
}

t.Run("generated namespace is invalid", func(t *testing.T) {
longAppID := "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/applications.core/applications/this-is-a-very-long-application-name-that-is-invalid"
longEnvID := "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/applications.core/environments/this-is-a-very-long-environment-name-that-is-invalid"

tCtx.MockSP.EXPECT().GetStorageClient(gomock.Any(), gomock.Any()).Return(tCtx.MockSC, nil).Times(1)

envdm := &datamodel.Environment{
Properties: datamodel.EnvironmentProperties{
Compute: rpv1.EnvironmentCompute{
Kind: rpv1.KubernetesComputeKind,
},
},
}

tCtx.MockSC.
EXPECT().
Get(gomock.Any(), gomock.Any()).
Return(rpctest.FakeStoreObject(envdm), nil)

newResource := &datamodel.Application{
Properties: datamodel.ApplicationProperties{
BasicResourceProperties: rpv1.BasicResourceProperties{
Environment: longEnvID,
},
},
}

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

resp, err := CreateAppScopedNamespace(ctx, newResource, nil, &opts)
require.NoError(t, err)
res := resp.(*rest.BadRequestResponse)

require.Equal(t, "Application namespace 'this-is-a-very-long-environment-name-that-is-invalid-this-is-a-very-long-application-name-that-is-invalid' could not be created: the combination of application and environment names is too long.", res.Body.Error.Message)
})

t.Run("invalid namespace", func(t *testing.T) {
tCtx.MockSC.EXPECT().
Query(gomock.Any(), gomock.Any()).
Expand Down

0 comments on commit 2ca32a6

Please sign in to comment.