-
Notifications
You must be signed in to change notification settings - Fork 14
[On hold] Refactored postgreSqlDatabases to handle secrets properly #78
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
Conversation
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Signed-off-by: Zach Casper <[email protected]>
Reshrahim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested some changes on removing secretName property as we decided to use resource name
Security/secrets/recipes/kubernetes/bicep/kubernetes-secrets.bicep
Outdated
Show resolved
Hide resolved
| environment: environment | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add an example for the credentials secrets resource declaration here?
Signed-off-by: Zach Casper <[email protected]>
brooke-hamilton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this ready for review? The title says [WIP] so maybe I'm doing this too soon. There is one comment below, and I'll do more review if this is ready.
| // Get the secret reference. Should be only a single connected resource. | ||
| var radiusConnectionsMap = context.resource.?connections ?? {} | ||
| var radiusConnectionList = items(radiusConnectionsMap) | ||
| var radiusFirstConnection = length(radiusConnectionList) > 0 ? radiusConnectionList[0].value : null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would the secret be the only connection? Or if there are multiple, why would it be the first connection? Maybe a comment here would help explain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the challenge with modeling the secret as a connection. @Reshrahim highlighted an inconsistency in how we are handling secrets.
@lakshmimsft's approach was to use connections from a database resource to a secret. We got some things for free, but I'll have to let her comment on what exactly the benefits of using connections was.
However, in the Containers Resource Type, @sk593 used the secretName and key which is more Kubernetes-like. This appears more straightforward since there can only be one secretName and key. Unlike connections which could be anything.
We should be consistent.
It's on hold. Blocked by radius-project/radius#10890 |
|
Closing in lieu of #95 |
Update: This PR is blocked on radius-project/radius#10890
Description
This pull request refactors and improves the configuration and deployment of PostgreSQL databases and Kubernetes secrets in the Radius platform. The goal is to establish a best practices pattern for other database resources. The main focus is on enforcing the use of external secrets for database credentials, updating documentation and schema to reflect this requirement, and enhancing the Kubernetes and Terraform recipes for better resource naming, labeling, and secret handling. Specifically:
string#53Contributor Checklist
rad resource-type showis correctenum: []required: []for every object property (not just the top-level properties)readOnly: true