TF Secret should be checked for alphanumeric characters at the end #6237
Comments
|
Thanks for finding this Yetkin. Looks like the secret ends with "..0fb" but the issue is consecutive characters "-.". I think we should prioritize fixing this. I have been thinking about about existing secret name generation approach and I don't think we really need the prefix, just the hash should be enough to uniquely identify secrets for resources: https://github.com/radius-project/radius/blob/main/pkg/recipes/terraform/config/backends/kubernetes.go#L91 Going to add the devops issue to the upcoming sprint. |
sk593
added a commit
that referenced
this issue
Sep 19, 2023
# 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))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug information
This is an edge case but I ran into this issue where the secret couldn't be created because of the following error. Please see the screenshot attached.
Steps to reproduce (required)
-Observed behavior (required)
Desired behavior (required)
Secret creation should check for this edge case.
Workaround (optional)
System information
rad Version (required)
Operating system (required)
Additional context
AB#9300
The text was updated successfully, but these errors were encountered: