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

Feature specs: Applications.Core/secretStores extended use cases #57

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

willtsai
Copy link
Contributor

Here are the feature specs for extending the use cases of Applications.Core/secretStores

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>
Copy link
Contributor

@superbeeny superbeeny left a 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.

resources/2024-07-secretstore-feature-spec.md Outdated Show resolved Hide resolved
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.
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on what you mean by extenders being correct today? My understanding is that users must continue to store secrets in plain text within the extender resource? Reference. Whereas for Dapr resources, users may reference secret stores as a part of the resource metadata.

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>
Copy link

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.

@github-actions github-actions bot added the Stale label Aug 20, 2024
Copy link

This pull request has been closed due to inactivity. Feel free to reopen if you are still working on it.

@github-actions github-actions bot closed this Aug 27, 2024
@willtsai willtsai reopened this Sep 16, 2024
Comment on lines +58 to +75
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
}
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Comment on lines +120 to +124
recipe: {
name: 'twilio'
}
secrets: {
+ password: {
Copy link
Contributor

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.

Copy link
Contributor Author

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)

Comment on lines +196 to +202
username: ''
resources: [
{ id: cosmosAccount.id }
]
secrets: {
+ connectionString: {
+ valueFrom: {
Copy link
Contributor

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.

Copy link
Contributor Author

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`

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?

Copy link
Contributor Author

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.

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.
Copy link
Contributor

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.
Copy link
Contributor

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.

Copy link
Contributor

@lakshmimsft lakshmimsft Sep 16, 2024

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.

Copy link

github-actions bot commented Oct 1, 2024

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.

@github-actions github-actions bot added the Stale label Oct 1, 2024
Copy link

github-actions bot commented Oct 9, 2024

This pull request has been closed due to inactivity. Feel free to reopen if you are still working on it.

@github-actions github-actions bot closed this Oct 9, 2024
@kachawla
Copy link
Contributor

kachawla commented Oct 9, 2024

@willtsai looks like this got auto closed. Are there any pending updates or is this PR mainly pending approval at this point?

@willtsai willtsai reopened this Oct 23, 2024
@github-actions github-actions bot removed the Stale label Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants