From 6d306a13676eefb5edf654c87f3862d38e7be56b Mon Sep 17 00:00:00 2001 From: Shruthi Kumar Date: Tue, 19 Sep 2023 14:35:58 -0700 Subject: [PATCH] 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 }