Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connection values created by Radius controller are double-encoded. #6540

Closed
rynowak opened this issue Oct 20, 2023 · 2 comments · Fixed by #6541
Closed

Connection values created by Radius controller are double-encoded. #6540

rynowak opened this issue Oct 20, 2023 · 2 comments · Fixed by #6541
Assignees
Labels
bug Something is broken or not working as expected

Comments

@rynowak
Copy link
Contributor

rynowak commented Oct 20, 2023

Bug information

Steps to reproduce (required)

Run the Radius Kubernetes tutorial that's in PR here: radius-project/docs#864

Observed behavior (required)

When you port-forward to the app, it will crash on the TODO page. The message in the logs looks like:

webapp-65859486d7-x6mzj webapp node:internal/errors:496
webapp-65859486d7-x6mzj webapp     ErrorCaptureStackTrace(err);
webapp-65859486d7-x6mzj webapp     ^
webapp-65859486d7-x6mzj webapp
webapp-65859486d7-x6mzj webapp TypeError [ERR_INVALID_URL]: Invalid URL
webapp-65859486d7-x6mzj webapp     at new NodeError (node:internal/errors:405:5)
webapp-65859486d7-x6mzj webapp     at new URL (node:internal/url:637:13)
webapp-65859486d7-x6mzj webapp     at RedisClient.parseURL (/usr/src/app/node_modules/@redis/client/dist/lib/client/index.js:115:76)
webapp-65859486d7-x6mzj webapp     at Commander._RedisClient_initiateOptions (/usr/src/app/node_modules/@redis/client/dist/lib/client/index.js:311:36)
webapp-65859486d7-x6mzj webapp     at new RedisClient (/usr/src/app/node_modules/@redis/client/dist/lib/client/index.js:77:148)
webapp-65859486d7-x6mzj webapp     at new Commander (/usr/src/app/node_modules/@redis/client/dist/lib/commander.js:44:13)
webapp-65859486d7-x6mzj webapp     at create (/usr/src/app/node_modules/@redis/client/dist/lib/client/index.js:111:16)
webapp-65859486d7-x6mzj webapp     at createClient (/usr/src/app/node_modules/redis/dist/index.js:38:38)
webapp-65859486d7-x6mzj webapp     at RedisFactory.<anonymous> (/usr/src/app/dist/db/redis.js:21:51)
webapp-65859486d7-x6mzj webapp     at Generator.next (<anonymous>) {
webapp-65859486d7-x6mzj webapp   input: 'cmVkaXM6Ly9yZWRpcy13d2d0a2M2MzY0azVjLmRlbW8uc3ZjLmNsdXN0ZXIubG9jYWw6NjM3OS8wPw',
webapp-65859486d7-x6mzj webapp   code: 'ERR_INVALID_URL'
webapp-65859486d7-x6mzj webapp }
webapp-65859486d7-x6mzj webapp
webapp-65859486d7-x6mzj webapp Node.js v18.18.2

Desired behavior (required)

Tutorial works.

Workaround (optional)

It's a tutorial. It works or it doesn't 😆

AB#9886

@rynowak rynowak added the bug Something is broken or not working as expected label Oct 20, 2023
@rynowak
Copy link
Contributor Author

rynowak commented Oct 20, 2023

The root cause of this is double-encoding.

You can see this in the UI provided by the application:

image

All of these values are the base64-encoded versions of the correct values. This UI should display these values in the plaintext if the configuration is correct.

The value stored in the secret created by the Radius controller is Y21Wa2FYTTZMeTl5WldScGN5MTNkMmQwYTJNMk16WTBhelZqTG1SbGJXOHVjM1pqTG1Oc2RYTjBaWEl1Ykc5allXdzZOak0zT1M4d1B3

.... which base64-decodes to cmVkaXM6Ly9yZWRpcy13d2d0a2M2MzY0azVjLmRlbW8uc3ZjLmNsdXN0ZXIubG9jYWw6NjM3OS8wPw

.... which base64-decodes to redis://redis-wwgtkc6364k5c.demo.svc.cluster.local:6379/0? (the correct value).

Somewhere along the way we're storing the value incorrectly in the secret.

@rynowak
Copy link
Contributor Author

rynowak commented Oct 20, 2023

The explanation of this behavior is here: https://github.com/weibeld/kubernetes-client-go-examples#ex5-secrets

Using the Kubernetes client from Go, you read and write secrets in their decoded form.

rynowak added a commit that referenced this issue Oct 20, 2023
Fixes: #6540

This change removes manual encoding from our interactions with `secret.Data`, which should address the double-encoding issue.

The problem here was a wrong assumption about the way `secret.Data` works in the Go Kubernetes client. This link documents the actual behavior: https://github.com/weibeld/kubernetes-client-go-examples/blob/master/README.md#ex5-secrets

I had a hint that something was wrong here while I was writing the code, but I wrongly assumed it was a quirk of the envtest library. The right behavior is to store the **decoded** value in `secret.Data`. Therefore the fix is to remove all of the places we encode data before storing it.
@rynowak rynowak self-assigned this Oct 20, 2023
rynowak added a commit that referenced this issue Oct 20, 2023
# Description


This change removes manual encoding from our interactions with
`secret.Data`, which should address the double-encoding issue.

The problem here was a wrong assumption about the way `secret.Data`
works in the Go Kubernetes client. This link documents the actual
behavior:
https://github.com/weibeld/kubernetes-client-go-examples/blob/master/README.md#ex5-secrets

I had a hint that something was wrong here while I was writing the code,
but I wrongly assumed it was a quirk of the envtest library. The right
behavior is to store the **decoded** value in `secret.Data`. Therefore
the fix is to remove all of the places we encode data before storing it.


## Type of change



- This pull request fixes a bug in Radius and has an approved issue
(issue link required).


Fixes: #6540

## Auto-generated summary

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

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

### Summary
🐛🧹🚀

<!--
1. 🐛 - This emoji represents a bug fix, which is the main purpose of the
first pull request. It shows that the test failure was resolved and the
unused import was removed.
2. 🧹 - This emoji represents a code cleanup or refactoring, which is the
main purpose of the second pull request. It shows that the code was
simplified, commented, and improved.
3. 🚀 - This emoji represents a performance improvement or optimization,
which is the main purpose of the third pull request. It shows that the
secret handling logic was made more efficient and correct.
-->
This pull request refactors and fixes the `reconciler` package, which
handles the creation and update of Kubernetes deployments and secrets
based on recipes. It removes unnecessary imports, encoding, and
complexity from the code and the tests.

> _Sing, O Muse, of the valiant code warriors who strove_
> _To mend the broken tests and reconcile the secrets_
> _That vexed the deployment function with double encoding_
> _And cluttered the package with imports that served no purpose_

### Walkthrough
* Remove unused imports of `encoding/base64` package from `reconciler`
package in `deployment_reconciler.go`, `deployment_reconciler_test.go`,
`recipe_reconciler.go`, and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL20),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL20))
* Simplify logic of `updateDeployment` and `updateSecret` functions by
removing unnecessary encoding of secret values using `base64` package in
`deployment_reconciler.go` and `recipe_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L556-R558),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L506-R505),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L517-R515))
* Add comment to explain why `updateDeployment` function uses `Data`
field of `Secret` object instead of `StringData` field in
`deployment_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195R519-R520))
* Update expected secret data in tests to match new logic of
`updateDeployment` and `updateSecret` functions in
`deployment_reconciler_test.go` and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL369-R371),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL415-R415),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL306-R306))
willdavsmith pushed a commit that referenced this issue Oct 23, 2023
# Description

This change removes manual encoding from our interactions with
`secret.Data`, which should address the double-encoding issue.

The problem here was a wrong assumption about the way `secret.Data`
works in the Go Kubernetes client. This link documents the actual
behavior:
https://github.com/weibeld/kubernetes-client-go-examples/blob/master/README.md#ex5-secrets

I had a hint that something was wrong here while I was writing the code,
but I wrongly assumed it was a quirk of the envtest library. The right
behavior is to store the **decoded** value in `secret.Data`. Therefore
the fix is to remove all of the places we encode data before storing it.

## Type of change

- This pull request fixes a bug in Radius and has an approved issue
(issue link required).

Fixes: #6540

## Auto-generated summary

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

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

### Summary
🐛🧹🚀

<!--
1. 🐛 - This emoji represents a bug fix, which is the main purpose of the
first pull request. It shows that the test failure was resolved and the
unused import was removed.
2. 🧹 - This emoji represents a code cleanup or refactoring, which is the
main purpose of the second pull request. It shows that the code was
simplified, commented, and improved.
3. 🚀 - This emoji represents a performance improvement or optimization,
which is the main purpose of the third pull request. It shows that the
secret handling logic was made more efficient and correct.
-->
This pull request refactors and fixes the `reconciler` package, which
handles the creation and update of Kubernetes deployments and secrets
based on recipes. It removes unnecessary imports, encoding, and
complexity from the code and the tests.

> _Sing, O Muse, of the valiant code warriors who strove_
> _To mend the broken tests and reconcile the secrets_
> _That vexed the deployment function with double encoding_
> _And cluttered the package with imports that served no purpose_

### Walkthrough
* Remove unused imports of `encoding/base64` package from `reconciler`
package in `deployment_reconciler.go`, `deployment_reconciler_test.go`,
`recipe_reconciler.go`, and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL20),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL20))
* Simplify logic of `updateDeployment` and `updateSecret` functions by
removing unnecessary encoding of secret values using `base64` package in
`deployment_reconciler.go` and `recipe_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L556-R558),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L506-R505),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L517-R515))
* Add comment to explain why `updateDeployment` function uses `Data`
field of `Secret` object instead of `StringData` field in
`deployment_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195R519-R520))
* Update expected secret data in tests to match new logic of
`updateDeployment` and `updateSecret` functions in
`deployment_reconciler_test.go` and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL369-R371),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL415-R415),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL306-R306))

(cherry picked from commit bf4e6bb)
willdavsmith pushed a commit that referenced this issue Oct 23, 2023
# Description

This change removes manual encoding from our interactions with
`secret.Data`, which should address the double-encoding issue.

The problem here was a wrong assumption about the way `secret.Data`
works in the Go Kubernetes client. This link documents the actual
behavior:
https://github.com/weibeld/kubernetes-client-go-examples/blob/master/README.md#ex5-secrets

I had a hint that something was wrong here while I was writing the code,
but I wrongly assumed it was a quirk of the envtest library. The right
behavior is to store the **decoded** value in `secret.Data`. Therefore
the fix is to remove all of the places we encode data before storing it.

## Type of change

- This pull request fixes a bug in Radius and has an approved issue
(issue link required).

Fixes: #6540

## Auto-generated summary

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

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

### Summary
🐛🧹🚀

<!--
1. 🐛 - This emoji represents a bug fix, which is the main purpose of the
first pull request. It shows that the test failure was resolved and the
unused import was removed.
2. 🧹 - This emoji represents a code cleanup or refactoring, which is the
main purpose of the second pull request. It shows that the code was
simplified, commented, and improved.
3. 🚀 - This emoji represents a performance improvement or optimization,
which is the main purpose of the third pull request. It shows that the
secret handling logic was made more efficient and correct.
-->
This pull request refactors and fixes the `reconciler` package, which
handles the creation and update of Kubernetes deployments and secrets
based on recipes. It removes unnecessary imports, encoding, and
complexity from the code and the tests.

> _Sing, O Muse, of the valiant code warriors who strove_
> _To mend the broken tests and reconcile the secrets_
> _That vexed the deployment function with double encoding_
> _And cluttered the package with imports that served no purpose_

### Walkthrough
* Remove unused imports of `encoding/base64` package from `reconciler`
package in `deployment_reconciler.go`, `deployment_reconciler_test.go`,
`recipe_reconciler.go`, and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL20),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL20))
* Simplify logic of `updateDeployment` and `updateSecret` functions by
removing unnecessary encoding of secret values using `base64` package in
`deployment_reconciler.go` and `recipe_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L556-R558),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L506-R505),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L517-R515))
* Add comment to explain why `updateDeployment` function uses `Data`
field of `Secret` object instead of `StringData` field in
`deployment_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195R519-R520))
* Update expected secret data in tests to match new logic of
`updateDeployment` and `updateSecret` functions in
`deployment_reconciler_test.go` and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL369-R371),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL415-R415),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL306-R306))

(cherry picked from commit bf4e6bb)
willdavsmith pushed a commit to willdavsmith/radius that referenced this issue Nov 3, 2023
# Description

This change removes manual encoding from our interactions with
`secret.Data`, which should address the double-encoding issue.

The problem here was a wrong assumption about the way `secret.Data`
works in the Go Kubernetes client. This link documents the actual
behavior:
https://github.com/weibeld/kubernetes-client-go-examples/blob/master/README.md#ex5-secrets

I had a hint that something was wrong here while I was writing the code,
but I wrongly assumed it was a quirk of the envtest library. The right
behavior is to store the **decoded** value in `secret.Data`. Therefore
the fix is to remove all of the places we encode data before storing it.

## Type of change

- This pull request fixes a bug in Radius and has an approved issue
(issue link required).

Fixes: radius-project#6540

## Auto-generated summary

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

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

### Summary
🐛🧹🚀

<!--
1. 🐛 - This emoji represents a bug fix, which is the main purpose of the
first pull request. It shows that the test failure was resolved and the
unused import was removed.
2. 🧹 - This emoji represents a code cleanup or refactoring, which is the
main purpose of the second pull request. It shows that the code was
simplified, commented, and improved.
3. 🚀 - This emoji represents a performance improvement or optimization,
which is the main purpose of the third pull request. It shows that the
secret handling logic was made more efficient and correct.
-->
This pull request refactors and fixes the `reconciler` package, which
handles the creation and update of Kubernetes deployments and secrets
based on recipes. It removes unnecessary imports, encoding, and
complexity from the code and the tests.

> _Sing, O Muse, of the valiant code warriors who strove_
> _To mend the broken tests and reconcile the secrets_
> _That vexed the deployment function with double encoding_
> _And cluttered the package with imports that served no purpose_

### Walkthrough
* Remove unused imports of `encoding/base64` package from `reconciler`
package in `deployment_reconciler.go`, `deployment_reconciler_test.go`,
`recipe_reconciler.go`, and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL20),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL20))
* Simplify logic of `updateDeployment` and `updateSecret` functions by
removing unnecessary encoding of secret values using `base64` package in
`deployment_reconciler.go` and `recipe_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L556-R558),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L506-R505),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L517-R515))
* Add comment to explain why `updateDeployment` function uses `Data`
field of `Secret` object instead of `StringData` field in
`deployment_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195R519-R520))
* Update expected secret data in tests to match new logic of
`updateDeployment` and `updateSecret` functions in
`deployment_reconciler_test.go` and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL369-R371),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL415-R415),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL306-R306))

Signed-off-by: willdavsmith <[email protected]>
willdavsmith pushed a commit that referenced this issue Nov 6, 2023
# Description

This change removes manual encoding from our interactions with
`secret.Data`, which should address the double-encoding issue.

The problem here was a wrong assumption about the way `secret.Data`
works in the Go Kubernetes client. This link documents the actual
behavior:
https://github.com/weibeld/kubernetes-client-go-examples/blob/master/README.md#ex5-secrets

I had a hint that something was wrong here while I was writing the code,
but I wrongly assumed it was a quirk of the envtest library. The right
behavior is to store the **decoded** value in `secret.Data`. Therefore
the fix is to remove all of the places we encode data before storing it.

## Type of change

- This pull request fixes a bug in Radius and has an approved issue
(issue link required).

Fixes: #6540

## Auto-generated summary

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

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

### Summary
🐛🧹🚀

<!--
1. 🐛 - This emoji represents a bug fix, which is the main purpose of the
first pull request. It shows that the test failure was resolved and the
unused import was removed.
2. 🧹 - This emoji represents a code cleanup or refactoring, which is the
main purpose of the second pull request. It shows that the code was
simplified, commented, and improved.
3. 🚀 - This emoji represents a performance improvement or optimization,
which is the main purpose of the third pull request. It shows that the
secret handling logic was made more efficient and correct.
-->
This pull request refactors and fixes the `reconciler` package, which
handles the creation and update of Kubernetes deployments and secrets
based on recipes. It removes unnecessary imports, encoding, and
complexity from the code and the tests.

> _Sing, O Muse, of the valiant code warriors who strove_
> _To mend the broken tests and reconcile the secrets_
> _That vexed the deployment function with double encoding_
> _And cluttered the package with imports that served no purpose_

### Walkthrough
* Remove unused imports of `encoding/base64` package from `reconciler`
package in `deployment_reconciler.go`, `deployment_reconciler_test.go`,
`recipe_reconciler.go`, and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL20),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L21),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL20))
* Simplify logic of `updateDeployment` and `updateSecret` functions by
removing unnecessary encoding of secret values using `base64` package in
`deployment_reconciler.go` and `recipe_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195L556-R558),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L506-R505),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-565f9bf43f08d107a3218cdeb8b17eab13363d0661a65535e80ec4ef114f09e5L517-R515))
* Add comment to explain why `updateDeployment` function uses `Data`
field of `Secret` object instead of `StringData` field in
`deployment_reconciler.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-87a7dfa06c174a6b41b671b2cfffb84c81e481881baec866a026e7dd00db8195R519-R520))
* Update expected secret data in tests to match new logic of
`updateDeployment` and `updateSecret` functions in
`deployment_reconciler_test.go` and `recipe_reconciler_test.go`
([link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL369-R371),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-9d66db7eb311e875d019923b0892c9b51b9520295624cc0b6a0197cdbd04527cL415-R415),
[link](https://github.com/radius-project/radius/pull/6541/files?diff=unified&w=0#diff-2b3aa3c5be32be791c9069d44213d653090aed157026f3d37f187ccded049cacL306-R306))

Signed-off-by: willdavsmith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken or not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant