Skip to content

Commit

Permalink
Removing TF secret prefix (#6273)
Browse files Browse the repository at this point in the history
# 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

<!--

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 is a minor refactor, code cleanup, test improvement,
or other maintenance task and doesn't change the functionality of Radius
#6237.

<!--

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

-->

Fixes: #6237

## Auto-generated summary

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

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

### Summary
:white_check_mark::wrench::whale:

<!--
1. :white_check_mark: - This emoji represents the addition and removal
of test cases, as well as the fact that the changes improve the test
coverage and quality of the `terraform` package.
2. :wrench: - This emoji represents the simplification and
standardization of the secret suffix generation logic, as well as the
fact that the changes fix some potential issues with the previous
implementation, such as collisions or truncation errors.
3. :whale: - This emoji represents the Kubernetes context of the
changes, as well as the fact that the changes affect the Terraform
backend configuration and secret management for Kubernetes resources.
-->
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))
  • Loading branch information
sk593 authored Sep 19, 2023
1 parent 9511b04 commit 6d306a1
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 40 deletions.
12 changes: 1 addition & 11 deletions pkg/recipes/terraform/config/backends/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,24 +71,14 @@ 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 {
return "", 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.
Expand Down
46 changes: 32 additions & 14 deletions pkg/recipes/terraform/config/backends/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ const (
testTemplatePath = "Azure/redis/azurerm"
testRecipeName = "redis-azure"
testTemplateVersion = "1.1.0"
envName = "env"
appName = "app"
resourceName = "redis"
)

var (
Expand Down Expand Up @@ -82,28 +85,43 @@ 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"
_, err := generateSecretSuffix(&resourceRecipe)
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")
}
28 changes: 13 additions & 15 deletions test/functional/shared/resources/recipe_terraform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ package resource_test

import (
"context"
"crypto/sha1"
"encoding/base64"
"encoding/json"
"fmt"
Expand All @@ -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"
Expand Down Expand Up @@ -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
}

0 comments on commit 6d306a1

Please sign in to comment.