-
Notifications
You must be signed in to change notification settings - Fork 12
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
Feature specs: Applications.Core/secretStores
extended use cases
#57
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>
Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>
Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>
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.
Looks good, just have the one question around naming conventions for types.
Signed-off-by: Will <28876888+willtsai@users.noreply.github.com>
|
||
As an operator, I can create a `secretStores` resource to securely manage secrets for use in the Radius Environments I provide for my developers, which is handy to allow for authentication into private Recipe registries and custom Terraform Providers. However, the `secretStores` resource type is not yet supported by the `Applications.Core/containers`, `Applications.Core/extenders`, `Applications.Core/volumes`, `Applications.Datastores/*`, and `Applications.Messaging/*` resource types. Thus, I have to do extra work to inject secrets as environment variables by configuring each custom Recipe in order to propagate secrets to my application developers. | ||
|
||
As an application developer, I cannot reference a `secretStores` resource in the `Applications.Core/containers`, `Applications.Core/extenders`, `Applications.Core/volumes`, `Applications.Datastores/*`, and `Applications.Messaging/*` resource types to securely manage secrets for use in my application. Instead, I'm having to store my secrets in plain text within the `properties.secrets` field for each resource. This is a critical gap in the secret management capabilities of Radius that is hindering my ability to securely manage secrets for use in my containers. |
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.
Instead, I'm having to store my secrets in plain text within the
properties.secrets
field for each resource.
We actually do this correctly for extender today. It's the Dapr resources that are wrong.
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.
I'm hoping we change the design for secret handling all-up for Radius as part of this effort and the UDT effort.
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.
Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>
Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>
Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>
This pull request is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
This pull request has been closed due to inactivity. Feel free to reopen if you are still working on it. |
resource azurekeyvaultsecrets 'Applications.Core/secretStores@2023-10-01-preview' = { | ||
name: 'azurekeyvaultsecrets' | ||
properties:{ | ||
application: application | ||
type: 'generic' // this can change based on detailed tech design | ||
data: { | ||
'name': { | ||
value: secret1 | ||
} | ||
'version': { | ||
value: 1 | ||
} | ||
'encoding': { | ||
value: 'base64' | ||
} | ||
'alias': { | ||
value: secretalias | ||
} |
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.
I couldn't tell how it references from azure keyvault, could you please elaborate on that?
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.
@kachawla yes, good catch, in this case I missed adding the reference to an external resource name, so I've added resource: 'secret-app-existing-secret'
to the example. Hopefully that clears it up.
recipe: { | ||
name: 'twilio' | ||
} | ||
secrets: { | ||
+ password: { |
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.
Are you proposing a new property "secrets" to specify recipe's input parameters that contain sensitive data? If so, I think this should be nested under recipe->parameters to make it more intuitive and consistent with rest of the parameters.
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.
Brooke pointed out some similar feedback, which is something I hadn't considered. I'll add it as an additional scenario (referencing secrets in recipes)
username: '' | ||
resources: [ | ||
{ id: cosmosAccount.id } | ||
] | ||
secrets: { | ||
+ connectionString: { | ||
+ valueFrom: { |
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.
We should revisit the overall plan for provisioning types we plan to support on portable resources before implementing this change. There are some changes with UDT that haven't been solidified yet.
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.
added this as a note to the feature spec
@@ -0,0 +1,286 @@ | |||
# Extend the use-cases of `Applications.Core/secretStores` |
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.
Does this design allow for secrets to be passed as parameters into a Radius Bicep resource?
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.
good question - that's currently a missing scenario, I'll be sure to add it!
|
||
### Detailed User Experience | ||
|
||
Step 1: Operator creates and deploys `Applications.Core/secretStores` resources containing the secret data required for authenticating into resources that their developers may use. |
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.
I think an additional likely scenario is that a dev team designs and radifies their application first, and then talks to an operations team about hosting it. The operators would then be able to create secrets in the environments they control in order to support the application.
However, I think the order of steps listed here is still legitimate and would also cover the scenario in which a dev team defines the required secrets.
|
||
### Feature 2: Add functionality to reference `Applications.Core/secretStores` in `Applications.Core/*` resources | ||
<!-- One or two sentence summary --> | ||
Add the ability for developers to reference values from their `Applications.Core/secretStores` resources in their core resources (namely `Applications.Core/extenders` and `Applications.Core/volumes`) so that secrets can be securely managed for use in their application to authenticate into those resources. |
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.
nit: Should extender be bucketed under portable resources (feature 3)?
|
||
> The TLS certificate data secret in `Applications.Core/gateways` is referenced today by `tls: { certificateFrom: secretstore.id }`. As a part of implementation we should evaluate if the `valueFrom: { secretRef: { ... } }` pattern proposed here is an acceptable deviation from the previous pattern implemented for `gateways`. | ||
|
||
**Risk: use of secrets in core and portable resources.** This pattern of referencing and leveraging secrets in core and portable resources might not be a common or desirable pattern for users. We will validate this by opening up this feature spec for discussion with the community. |
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.
Not sure if I follow this, are there alternatives that we think might be more desirable?
As an operator, I can create a `secretStores` resource to securely manage secrets for use in the Radius Environments I provide for my developers, which is handy to allow for authentication into private Recipe registries and custom Terraform Providers. However, the `secretStores` resource type is not yet supported by the `Applications.Core/containers`, `Applications.Core/extenders`, `Applications.Core/volumes`, `Applications.Datastores/*`, and `Applications.Messaging/*` resource types. Thus, I have to do extra work to inject secrets as environment variables by configuring each custom Recipe in order to propagate secrets to my application developers. | ||
|
||
As an application developer, I cannot reference a `secretStores` resource in the `Applications.Core/containers`, `Applications.Core/extenders`, `Applications.Core/volumes`, `Applications.Datastores/*`, and `Applications.Messaging/*` resource types to securely manage secrets for use in my application. Instead, I'm having to store my secrets in plain text within the `properties.secrets` field for each resource. This is a critical gap in the secret management capabilities of Radius that is hindering my ability to securely manage secrets for use in my containers. | ||
|
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.
Requesting more info (as discussed) on adding more information on customers bringing in their own secrets, secrets stores resource currently supporting only k8s secrets.
This pull request is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
This pull request has been closed due to inactivity. Feel free to reopen if you are still working on it. |
@willtsai looks like this got auto closed. Are there any pending updates or is this PR mainly pending approval at this point? |
Here are the feature specs for extending the use cases of
Applications.Core/secretStores