From 9511b04f4fa34bae63966fc9a94f6027a4410a63 Mon Sep 17 00:00:00 2001 From: Aaron Crawfis Date: Mon, 18 Sep 2023 14:21:08 -0700 Subject: [PATCH 1/2] Update error message for missing application (#6281) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Adds an additional error message to `rad resource list`. ## Type of change - This pull request adds or changes features of Radius and has an approved issue (issue link required). Fixes: #6236 ## Auto-generated summary ### ๐Ÿค– Generated by Copilot at 4cbfd98 ### Summary ๐Ÿงช๐Ÿ“๐Ÿš€ Improve error handling and testing for `resource list` command. Add a clearer error message for invalid applications and update the corresponding test case. > _`Run` test updated_ > _Error message more helpful_ > _Autumn of bug fixes_ ### Walkthrough * Improve error message for invalid application name in `list` command ([link](https://github.com/radius-project/radius/pull/6281/files?diff=unified&w=0#diff-b21dcd67dbc0d24e0f8bba90701dfcfb5736cf40427547c7a7ccb91478f262cfL160-R160)) * Update test case for `Run` function in `list_test.go` to match new error message ([link](https://github.com/radius-project/radius/pull/6281/files?diff=unified&w=0#diff-d57c4c1535dd3783fc94937211b66a786a621683678bcefdef775626c9e22c5eL125-R125)) --- pkg/cli/cmd/resource/list/list.go | 2 +- pkg/cli/cmd/resource/list/list_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cli/cmd/resource/list/list.go b/pkg/cli/cmd/resource/list/list.go index dfdc097143..f931d6ff53 100644 --- a/pkg/cli/cmd/resource/list/list.go +++ b/pkg/cli/cmd/resource/list/list.go @@ -157,7 +157,7 @@ func (r *Runner) Run(ctx context.Context) error { } else { _, err = client.ShowApplication(ctx, r.ApplicationName) if clients.Is404Error(err) { - return clierrors.Message("The application %q could not be found in workspace %q.", r.ApplicationName, r.Workspace.Name) + return clierrors.Message("The application %q could not be found in workspace %q. Make sure you specify the correct application with '-a/--application' or switch applications with 'rad app switch'.", r.ApplicationName, r.Workspace.Name) } else if err != nil { return err } diff --git a/pkg/cli/cmd/resource/list/list_test.go b/pkg/cli/cmd/resource/list/list_test.go index 9ab7634129..dba2e4822f 100644 --- a/pkg/cli/cmd/resource/list/list_test.go +++ b/pkg/cli/cmd/resource/list/list_test.go @@ -122,7 +122,7 @@ func Test_Run(t *testing.T) { err := runner.Run(context.Background()) require.Error(t, err) - require.IsType(t, err, clierrors.Message("The application %q could not be found in workspace %q.", "test-app", radcli.TestWorkspaceName)) + require.IsType(t, err, clierrors.Message("The application %q could not be found in workspace %q. Make sure you specify the correct application with '-a/--application' or switch applications with 'rad app switch'.", "test-app", radcli.TestWorkspaceName)) }) t.Run("Success", func(t *testing.T) { From 6d306a13676eefb5edf654c87f3862d38e7be56b Mon Sep 17 00:00:00 2001 From: Shruthi Kumar Date: Tue, 19 Sep 2023 14:35:58 -0700 Subject: [PATCH 2/2] Removing TF secret prefix (#6273) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description This updates the kubernetes secret generation for TF to only use the hash based on the env, app, and resource names (previously we prefixed the hash with the env/app/resource name as well) ## Type of change - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius #6237. Fixes: #6237 ## Auto-generated summary ### ๐Ÿค– Generated by Copilot at eea0445 ### Summary :white_check_mark::wrench::whale: This pull request improves the testing and implementation of the `terraform` package's Kubernetes backend. It updates the test cases in `kubernetes_test.go` and simplifies the secret suffix generation in `kubernetes.go` by using a hex hash. > _Sing, O Muse, of the skillful engineers who changed_ > _The code of Terraform, the mighty tool of cloud_ > _They tested well their work, and added and removed_ > _The cases that would prove their backend sound and proud_ ### Walkthrough * Simplify and improve secret suffix generation for Kubernetes backend ([link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-8cfcf199134239080517722934e076294829b01cc0c3dbd971f22f75818e6ed2L74-L82), [link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-8cfcf199134239080517722934e076294829b01cc0c3dbd971f22f75818e6ed2L89-R80), [link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-98512ebd0fd586cfa9b266a18f807392d48df130f68f8d483ff71f002c1e6e0cL92-R126)) - Remove truncation logic and use only hex hash of lowercased names ([link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-8cfcf199134239080517722934e076294829b01cc0c3dbd971f22f75818e6ed2L74-L82), [link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-8cfcf199134239080517722934e076294829b01cc0c3dbd971f22f75818e6ed2L89-R80)) - Remove obsolete test case and add new ones for error scenarios ([link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-98512ebd0fd586cfa9b266a18f807392d48df130f68f8d483ff71f002c1e6e0cL92-R126)) * Add constants and new test cases for Kubernetes backend config ([link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-98512ebd0fd586cfa9b266a18f807392d48df130f68f8d483ff71f002c1e6e0cR34-R36), [link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-98512ebd0fd586cfa9b266a18f807392d48df130f68f8d483ff71f002c1e6e0cR88-R107)) - Define constants for environment, application and resource names ([link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-98512ebd0fd586cfa9b266a18f807392d48df130f68f8d483ff71f002c1e6e0cR34-R36)) - Test error scenario when service host and port are not set ([link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-98512ebd0fd586cfa9b266a18f807392d48df130f68f8d483ff71f002c1e6e0cR88-R107)) - Test happy path scenario when secret suffix is generated correctly ([link](https://github.com/radius-project/radius/pull/6273/files?diff=unified&w=0#diff-98512ebd0fd586cfa9b266a18f807392d48df130f68f8d483ff71f002c1e6e0cR88-R107)) --- .../terraform/config/backends/kubernetes.go | 12 +---- .../config/backends/kubernetes_test.go | 46 +++++++++++++------ .../shared/resources/recipe_terraform_test.go | 28 ++++++----- 3 files changed, 46 insertions(+), 40 deletions(-) diff --git a/pkg/recipes/terraform/config/backends/kubernetes.go b/pkg/recipes/terraform/config/backends/kubernetes.go index e91de84c80..08115a63f3 100644 --- a/pkg/recipes/terraform/config/backends/kubernetes.go +++ b/pkg/recipes/terraform/config/backends/kubernetes.go @@ -71,15 +71,6 @@ func generateSecretSuffix(resourceRecipe *recipes.ResourceMetadata) (string, err return "", err } - prefix := fmt.Sprintf("%s-%s-%s", parsedEnvID.Name(), parsedAppID.Name(), parsedResourceID.Name()) - - // Kubernetes enforces a character limit of 63 characters on the suffix for state file stored in kubernetes secret. - // 22 = 63 (max length of Kubernetes secret suffix) - 40 (hex hash length) - 1 (dot separator) - maxResourceNameLen := 22 - if len(prefix) >= maxResourceNameLen { - prefix = prefix[:maxResourceNameLen] - } - hasher := sha1.New() _, err = hasher.Write([]byte(strings.ToLower(fmt.Sprintf("%s-%s-%s", parsedEnvID.Name(), parsedAppID.Name(), parsedResourceID.String())))) if err != nil { @@ -87,8 +78,7 @@ func generateSecretSuffix(resourceRecipe *recipes.ResourceMetadata) (string, err } hash := hasher.Sum(nil) - // example: env-app-redis.ec291e26078b7ea8a74abfac82530005a0ecbf15 - return fmt.Sprintf("%s.%x", prefix, hash), nil + return fmt.Sprintf("%x", hash), nil } // generateKubernetesBackendConfig returns Terraform backend configuration to store Terraform state file for the deployment. diff --git a/pkg/recipes/terraform/config/backends/kubernetes_test.go b/pkg/recipes/terraform/config/backends/kubernetes_test.go index 0ad6be122f..9efe0b3ab8 100644 --- a/pkg/recipes/terraform/config/backends/kubernetes_test.go +++ b/pkg/recipes/terraform/config/backends/kubernetes_test.go @@ -31,6 +31,9 @@ const ( testTemplatePath = "Azure/redis/azurerm" testRecipeName = "redis-azure" testTemplateVersion = "1.1.0" + envName = "env" + appName = "app" + resourceName = "redis" ) var ( @@ -82,6 +85,26 @@ func Test_GenerateKubernetesBackendConfig(t *testing.T) { require.Equal(t, expectedConfig, actualConfig) } +func Test_GenerateKubernetesBackendConfig_Error(t *testing.T) { + t.Setenv("KUBERNETES_SERVICE_HOST", "testvalue") + t.Setenv("KUBERNETES_SERVICE_PORT", "1111") + + backend, err := generateKubernetesBackendConfig("test-suffix") + require.Error(t, err) + require.Nil(t, backend) +} + +func Test_GenerateSecretSuffix(t *testing.T) { + _, resourceRecipe := getTestInputs() + hasher := sha1.New() + _, err := hasher.Write([]byte(strings.ToLower(fmt.Sprintf("%s-%s-%s", envName, appName, resourceRecipe.ResourceID)))) + require.NoError(t, err) + expSecret := fmt.Sprintf("%x", hasher.Sum(nil)) + secret, err := generateSecretSuffix(&resourceRecipe) + require.NoError(t, err) + require.Equal(t, expSecret, secret) +} + func Test_GenerateSecretSuffix_invalid_resourceid(t *testing.T) { _, resourceRecipe := getTestInputs() resourceRecipe.ResourceID = "invalid" @@ -89,21 +112,16 @@ func Test_GenerateSecretSuffix_invalid_resourceid(t *testing.T) { require.Equal(t, err.Error(), "'invalid' is not a valid resource id") } -func Test_GenerateSecretSuffix_with_lengthy_resource_name(t *testing.T) { +func Test_GenerateSecretSuffix_invalid_envid(t *testing.T) { _, resourceRecipe := getTestInputs() - act, err := generateSecretSuffix(&resourceRecipe) - require.NoError(t, err) - hasher := sha1.New() - _, _ = hasher.Write([]byte(strings.ToLower("env-app-" + resourceRecipe.ResourceID))) - hash := hasher.Sum(nil) - require.Equal(t, act, "env-app-redis."+fmt.Sprintf("%x", hash)) + resourceRecipe.EnvironmentID = "invalid" + _, err := generateSecretSuffix(&resourceRecipe) + require.Equal(t, err.Error(), "'invalid' is not a valid resource id") } -func Test_GenerateKubernetesBackendConfig_Error(t *testing.T) { - t.Setenv("KUBERNETES_SERVICE_HOST", "testvalue") - t.Setenv("KUBERNETES_SERVICE_PORT", "1111") - - backend, err := generateKubernetesBackendConfig("test-suffix") - require.Error(t, err) - require.Nil(t, backend) +func Test_GenerateSecretSuffix_invalid_appid(t *testing.T) { + _, resourceRecipe := getTestInputs() + resourceRecipe.ApplicationID = "invalid" + _, err := generateSecretSuffix(&resourceRecipe) + require.Equal(t, err.Error(), "'invalid' is not a valid resource id") } diff --git a/test/functional/shared/resources/recipe_terraform_test.go b/test/functional/shared/resources/recipe_terraform_test.go index ea4368c500..e0c13cc0a9 100644 --- a/test/functional/shared/resources/recipe_terraform_test.go +++ b/test/functional/shared/resources/recipe_terraform_test.go @@ -26,7 +26,6 @@ package resource_test import ( "context" - "crypto/sha1" "encoding/base64" "encoding/json" "fmt" @@ -39,6 +38,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/radius-project/radius/pkg/recipes" + "github.com/radius-project/radius/pkg/recipes/terraform/config/backends" "github.com/radius-project/radius/pkg/ucp/resources" resources_radius "github.com/radius-project/radius/pkg/ucp/resources/radius" "github.com/radius-project/radius/test/functional" @@ -382,24 +382,22 @@ func testSecretDeletion(t *testing.T, ctx context.Context, test shared.RPTest, a } func getSecretSuffix(resourceID, envName, appName string) (string, error) { - parsedResourceID, err := resources.Parse(resourceID) - if err != nil { - return "", err - } - - prefix := fmt.Sprintf("%s-%s-%s", envName, appName, parsedResourceID.Name()) - maxResourceNameLen := 22 - if len(prefix) >= maxResourceNameLen { - prefix = prefix[:maxResourceNameLen] + envID := "/planes/radius/local/resourcegroups/kind-radius/providers/Applications.Core/environments/" + envName + appID := "/planes/radius/local/resourcegroups/kind-radius/providers/Applications.Core/applications/" + appName + + resourceRecipe := recipes.ResourceMetadata{ + EnvironmentID: envID, + ApplicationID: appID, + ResourceID: resourceID, + Parameters: nil, } - hasher := sha1.New() - _, err = hasher.Write([]byte(strings.ToLower(fmt.Sprintf("%s-%s-%s", envName, appName, parsedResourceID.String())))) + backend := backends.NewKubernetesBackend() + secretMap, err := backend.BuildBackend(&resourceRecipe) if err != nil { return "", err } - hash := hasher.Sum(nil) + kubernetes := secretMap["kubernetes"].(map[string]any) - // example: env-app-redis.ec291e26078b7ea8a74abfac82530005a0ecbf15 - return fmt.Sprintf("%s.%x", prefix, hash), nil + return kubernetes["secret_suffix"].(string), nil }