diff --git a/deploy/manifest/built-in-providers/dev/secrets.yaml b/deploy/manifest/built-in-providers/dev/secrets.yaml index 9f8151a4af6..47b9d0cda80 100644 --- a/deploy/manifest/built-in-providers/dev/secrets.yaml +++ b/deploy/manifest/built-in-providers/dev/secrets.yaml @@ -77,6 +77,7 @@ types: value: type: string x-radius-sensitive: true + x-radius-retain: true description: (Required) The string value of the secret unless encoding is set to 'base64'. required: [value] required: diff --git a/deploy/manifest/built-in-providers/self-hosted/secrets.yaml b/deploy/manifest/built-in-providers/self-hosted/secrets.yaml index 9f8151a4af6..47b9d0cda80 100644 --- a/deploy/manifest/built-in-providers/self-hosted/secrets.yaml +++ b/deploy/manifest/built-in-providers/self-hosted/secrets.yaml @@ -77,6 +77,7 @@ types: value: type: string x-radius-sensitive: true + x-radius-retain: true description: (Required) The string value of the secret unless encoding is set to 'base64'. required: [value] required: diff --git a/pkg/corerp/api/v20250801preview/recipepack_conversion.go b/pkg/corerp/api/v20250801preview/recipepack_conversion.go index 0089d7d3226..4aeb87d90d5 100644 --- a/pkg/corerp/api/v20250801preview/recipepack_conversion.go +++ b/pkg/corerp/api/v20250801preview/recipepack_conversion.go @@ -103,6 +103,9 @@ func toRecipesDataModel(recipes map[string]*RecipeDefinition) map[string]*datamo if recipe.Outputs != nil { definition.Outputs = to.StringMap(recipe.Outputs) } + if recipe.SecretOutputs != nil { + definition.SecretOutputs = toSecretOutputsDataModel(recipe.SecretOutputs) + } result[key] = definition } } @@ -126,12 +129,48 @@ func fromRecipesDataModel(recipes map[string]*datamodel.RecipeDefinition) map[st if recipe.Outputs != nil { definition.Outputs = *to.StringMapPtr(recipe.Outputs) } + if recipe.SecretOutputs != nil { + definition.SecretOutputs = fromSecretOutputsDataModel(recipe.SecretOutputs) + } result[key] = definition } } return result } +// toSecretOutputsDataModel converts a versioned secretOutputs map (property name -> secret data key -> +// module output name, with pointer values) into the version-agnostic datamodel representation. +func toSecretOutputsDataModel(in map[string]map[string]*string) map[string]map[string]string { + if in == nil { + return nil + } + out := make(map[string]map[string]string, len(in)) + for property, keys := range in { + inner := make(map[string]string, len(keys)) + for secretKey, outputName := range keys { + inner[secretKey] = to.String(outputName) + } + out[property] = inner + } + return out +} + +// fromSecretOutputsDataModel converts a datamodel secretOutputs map into the versioned representation. +func fromSecretOutputsDataModel(in map[string]map[string]string) map[string]map[string]*string { + if in == nil { + return nil + } + out := make(map[string]map[string]*string, len(in)) + for property, keys := range in { + inner := make(map[string]*string, len(keys)) + for secretKey, outputName := range keys { + inner[secretKey] = to.Ptr(outputName) + } + out[property] = inner + } + return out +} + func toRecipeKindDataModel(kind *RecipeKind) string { if kind == nil { return "" diff --git a/pkg/corerp/api/v20250801preview/recipepack_conversion_test.go b/pkg/corerp/api/v20250801preview/recipepack_conversion_test.go index 84f33f81968..6dda1b29434 100644 --- a/pkg/corerp/api/v20250801preview/recipepack_conversion_test.go +++ b/pkg/corerp/api/v20250801preview/recipepack_conversion_test.go @@ -122,3 +122,30 @@ func TestRecipePackConvertInvalidModel(t *testing.T) { require.Equal(t, v1.ErrInvalidModelConversion, err) }) } + +func TestSecretOutputsConversion(t *testing.T) { + t.Run("round-trips a nested secretOutputs map", func(t *testing.T) { + versioned := map[string]map[string]*string{ + "secretName": { + "CONNECTIONSTRING": to.Ptr("primaryConnectionString"), + "HOST": to.Ptr("host"), + }, + } + + dm := toSecretOutputsDataModel(versioned) + require.Equal(t, map[string]map[string]string{ + "secretName": { + "CONNECTIONSTRING": "primaryConnectionString", + "HOST": "host", + }, + }, dm) + + back := fromSecretOutputsDataModel(dm) + require.Equal(t, versioned, back) + }) + + t.Run("nil maps convert to nil", func(t *testing.T) { + require.Nil(t, toSecretOutputsDataModel(nil)) + require.Nil(t, fromSecretOutputsDataModel(nil)) + }) +} diff --git a/pkg/corerp/api/v20250801preview/zz_generated_models.go b/pkg/corerp/api/v20250801preview/zz_generated_models.go index 787754e2915..764044de5c4 100644 --- a/pkg/corerp/api/v20250801preview/zz_generated_models.go +++ b/pkg/corerp/api/v20250801preview/zz_generated_models.go @@ -436,6 +436,12 @@ type RecipeDefinition struct { // Connect to the source using HTTP (not HTTPS). This should be used when the source is known not to support HTTPS, for example // in a locally hosted registry for Bicep recipes. Defaults to false (use HTTPS/TLS) PlainHTTP *bool + + // Map of resource type property names to a map of Kubernetes Secret data keys to module output names. Used for recipes that + // point directly at a Bicep or Terraform module to materialize sensitive module outputs into a Kubernetes Secret in the application's + // namespace. For each entry, Radius creates a Secret whose data keys are populated from the named module outputs, and sets + // the named resource property to the generated Secret's name so connected resources can reference it. + SecretOutputs map[string]map[string]*string } // RecipePackProperties - Recipe Pack properties diff --git a/pkg/corerp/api/v20250801preview/zz_generated_models_serde.go b/pkg/corerp/api/v20250801preview/zz_generated_models_serde.go index 136c75c8c15..09dbc877ebf 100644 --- a/pkg/corerp/api/v20250801preview/zz_generated_models_serde.go +++ b/pkg/corerp/api/v20250801preview/zz_generated_models_serde.go @@ -1005,6 +1005,7 @@ func (r RecipeDefinition) MarshalJSON() ([]byte, error) { populate(objectMap, "outputs", r.Outputs) populate(objectMap, "parameters", r.Parameters) populate(objectMap, "plainHttp", r.PlainHTTP) + populate(objectMap, "secretOutputs", r.SecretOutputs) populate(objectMap, "source", r.Source) return json.Marshal(objectMap) } @@ -1030,6 +1031,9 @@ func (r *RecipeDefinition) UnmarshalJSON(data []byte) error { case "plainHttp": err = unpopulate(val, "PlainHTTP", &r.PlainHTTP) delete(rawMsg, key) + case "secretOutputs": + err = unpopulate(val, "SecretOutputs", &r.SecretOutputs) + delete(rawMsg, key) case "source": err = unpopulate(val, "Source", &r.Source) delete(rawMsg, key) diff --git a/pkg/corerp/datamodel/recipepack.go b/pkg/corerp/datamodel/recipepack.go index 82ebfc8f75a..237d150d6e6 100644 --- a/pkg/corerp/datamodel/recipepack.go +++ b/pkg/corerp/datamodel/recipepack.go @@ -62,6 +62,11 @@ type RecipeDefinition struct { // directly at a Bicep or Terraform module to map the module's outputs onto resource properties. Outputs map[string]string `json:"outputs,omitempty"` + // SecretOutputs maps resource-type property names to a map of Kubernetes Secret data keys to + // module output names. Used to materialize sensitive module outputs into a Kubernetes Secret in + // the application's namespace; the named resource property is set to the generated Secret's name. + SecretOutputs map[string]map[string]string `json:"secretOutputs,omitempty"` + // PlainHTTP connects to the source using HTTP (not-HTTPS). PlainHTTP bool `json:"plainHTTP,omitempty"` } diff --git a/pkg/dynamicrp/frontend/encryptionfilter_test.go b/pkg/dynamicrp/frontend/encryptionfilter_test.go index 29b41f71d73..f98bad2bd37 100644 --- a/pkg/dynamicrp/frontend/encryptionfilter_test.go +++ b/pkg/dynamicrp/frontend/encryptionfilter_test.go @@ -277,6 +277,30 @@ func testUCPClientFactoryWithNestedSensitiveFields() (*v20231001preview.ClientFa }) } +// testUCPClientFactoryWithRetainFields returns a schema with one retain field (password: sensitive + +// retain) and one sensitive-only field (apikey: sensitive). Retain fields keep their encrypted value at +// rest and must be redacted on read even at Succeeded; sensitive-only fields are already nil at rest at +// Succeeded and are only redacted in non-terminal states. +func testUCPClientFactoryWithRetainFields() (*v20231001preview.ClientFactory, error) { + return createFakeUCPClientFactory(map[string]any{ + "type": "object", + "properties": map[string]any{ + "name": map[string]any{ + "type": "string", + }, + "password": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + "x-radius-retain": true, + }, + "apikey": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }) +} + func testUCPClientFactoryWithError() (*v20231001preview.ClientFactory, error) { apiVersionsServer := fake.APIVersionsServer{ Get: func(ctx context.Context, planeName, resourceProviderName, resourceTypeName, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (resp azfake.Responder[v20231001preview.APIVersionsClientGetResponse], errResp azfake.ErrorResponder) { diff --git a/pkg/dynamicrp/frontend/getresource.go b/pkg/dynamicrp/frontend/getresource.go index e19bb42a9ed..5dc7204d387 100644 --- a/pkg/dynamicrp/frontend/getresource.go +++ b/pkg/dynamicrp/frontend/getresource.go @@ -49,11 +49,13 @@ func NewGetResourceWithRedaction( // Run returns the requested resource with sensitive fields redacted. // -// Design consideration (GET Operation Update): When provisioningState is "Succeeded", -// the backend has already redacted sensitive data from the database, so we skip the -// schema fetch and redaction (fast path). For all other states (e.g., "Updating", -// "Accepted", "Failed"), the resource may still contain encrypted data, so we fetch -// the schema and redact sensitive fields to prevent exposure. +// Redaction is schema-driven. When provisioningState is "Succeeded" the backend has already redacted +// every non-retain sensitive field to nil, but retain fields (x-radius-retain, e.g. the secret value +// of Radius.Security/secrets) are persisted encrypted at rest so the secrets loader can decrypt them +// from the store. Those retain fields must be redacted on read so the API never returns the retained +// ciphertext. For all other states the resource may still contain encrypted data for any sensitive +// field, so every sensitive field is redacted. Because retain fields can survive into Succeeded, the +// schema is fetched on every read (the previous Succeeded fast-path that skipped the fetch is gone). func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWriter, req *http.Request) (rest.Response, error) { serviceCtx := v1.ARMRequestContextFromContext(ctx) logger := ucplog.FromContextOrDiscard(ctx) @@ -66,10 +68,7 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite return rest.NewNotFoundResponse(serviceCtx.ResourceID), nil } - // Fast path: if provisioningState is Succeeded, the backend has already redacted - // sensitive fields. Skip the schema fetch for better performance. - provisioningState := resource.ProvisioningState() - if provisioningState != v1.ProvisioningStateSucceeded && resource.Properties != nil { + if resource.Properties != nil { resourceID := serviceCtx.ResourceID.String() resourceType := serviceCtx.ResourceID.Type() @@ -77,15 +76,9 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite // encryption and redaction use the same schema apiVersion := resource.InternalMetadata.UpdatedAPIVersion - sensitiveFieldPaths, err := schema.GetSensitiveFieldPaths( - ctx, - c.ucpClient, - resourceID, - resourceType, - apiVersion, - ) + paths, err := fetchRedactionPaths(ctx, c.ucpClient, resourceID, resourceType, apiVersion) if err != nil { - logger.Error(err, "Failed to fetch sensitive field paths for GET redaction", + logger.Error(err, "Failed to fetch field paths for GET redaction", "resourceType", resourceType, "apiVersion", apiVersion) // Fail-safe: return error to prevent potential exposure of sensitive data // This is consistent with the write path (encryption filter) @@ -97,11 +90,13 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite }), nil } - if len(sensitiveFieldPaths) > 0 { - schema.RedactFields(resource.Properties, sensitiveFieldPaths) - logger.V(ucplog.LevelDebug).Info("Redacted sensitive fields in GET response", + provisioningState := resource.ProvisioningState() + fieldPaths := paths.forState(provisioningState) + if len(fieldPaths) > 0 { + schema.RedactFields(resource.Properties, fieldPaths) + logger.V(ucplog.LevelDebug).Info("Redacted fields in GET response", "provisioningState", provisioningState, - "count", len(sensitiveFieldPaths), "resourceType", resourceType) + "count", len(fieldPaths), "resourceType", resourceType) } } diff --git a/pkg/dynamicrp/frontend/getresource_test.go b/pkg/dynamicrp/frontend/getresource_test.go index 4ff2aa0b8e7..eb5cd5431b3 100644 --- a/pkg/dynamicrp/frontend/getresource_test.go +++ b/pkg/dynamicrp/frontend/getresource_test.go @@ -71,6 +71,53 @@ func newGetTestDynamicResource(provisioningState v1.ProvisioningState, propertie } } +func TestGetResourceWithRedaction_SucceededRedactsRetainOnly(t *testing.T) { + // At Succeeded a retain field still holds its encrypted value, so it MUST be redacted on read. A + // sensitive-only field was already redacted to nil at rest by the backend, so it is returned as-is + // (here, absent because the stored resource no longer carries it). + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newGetTestDynamicResource(v1.ProvisioningStateSucceeded, map[string]any{ + "password": "still-encrypted", + "apikey": "already-nil-at-rest", + }) + + storeObject := rpctest.FakeStoreObject(resource) + storeObject.Metadata = database.Metadata{ID: testResourceID, ETag: "etag-1"} + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Get(gomock.Any(), testResourceID). + Return(storeObject, nil) + + ucpClient, err := testUCPClientFactoryWithRetainFields() + require.NoError(t, err) + + c := newTestGetController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testGetURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + _ = resp.Apply(ctx, w, req) + require.Equal(t, http.StatusOK, w.Result().StatusCode) + + var body map[string]any + require.NoError(t, json.NewDecoder(w.Body).Decode(&body)) + properties, ok := body["properties"].(map[string]any) + require.True(t, ok) + // Retain field is redacted even at Succeeded. + require.Nil(t, properties["password"]) + // Sensitive-only field is not redacted at Succeeded (it is already nil at rest in practice). + require.Equal(t, "already-nil-at-rest", properties["apikey"]) +} + func TestGetResourceWithRedaction_NonSucceededRedacts(t *testing.T) { mctrl := gomock.NewController(t) defer mctrl.Finish() diff --git a/pkg/dynamicrp/frontend/listresources.go b/pkg/dynamicrp/frontend/listresources.go index 921189a3cc1..383b8757fec 100644 --- a/pkg/dynamicrp/frontend/listresources.go +++ b/pkg/dynamicrp/frontend/listresources.go @@ -66,9 +66,9 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri return nil, err } - // Cache sensitive field paths per API version - // Different resources in the list may have been created with different API versions - sensitiveFieldPathsCache := make(map[string][]string) + // Cache redaction paths per API version. + // Different resources in the list may have been created with different API versions. + redactionPathsCache := make(map[string]redactionPaths) items := []any{} for _, item := range result.Items { @@ -77,19 +77,18 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri return nil, err } - // Redact sensitive fields before adding to the response. - // Fast path: if provisioningState is Succeeded, the backend has already redacted - // sensitive fields. Skip redaction for these items. - provisioningState := resource.ProvisioningState() - if provisioningState != v1.ProvisioningStateSucceeded && resource.Properties != nil { + // Redact fields before adding to the response. Retain fields (x-radius-retain) survive into + // Succeeded as ciphertext and must be redacted on read, so unlike sensitive-only redaction we + // can't skip Succeeded items; we fetch the schema for every item with properties. + if resource.Properties != nil { // Use the API version the resource was last updated with to ensure // encryption and redaction use the same schema apiVersion := resource.InternalMetadata.UpdatedAPIVersion // Check cache first to avoid redundant schema fetches for same API version - sensitiveFieldPaths, cached := sensitiveFieldPathsCache[apiVersion] + paths, cached := redactionPathsCache[apiVersion] if !cached { - sensitiveFieldPaths, err = schema.GetSensitiveFieldPaths( + paths, err = fetchRedactionPaths( ctx, c.ucpClient, resource.ID, @@ -97,7 +96,7 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri apiVersion, ) if err != nil { - logger.Error(err, "Failed to fetch sensitive field paths for LIST redaction", + logger.Error(err, "Failed to fetch field paths for LIST redaction", "resourceType", serviceCtx.ResourceID.Type(), "apiVersion", apiVersion) // Fail-safe: return error to prevent potential exposure of sensitive data // This is consistent with the write path (encryption filter) @@ -108,11 +107,12 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri }, }), nil } - sensitiveFieldPathsCache[apiVersion] = sensitiveFieldPaths + redactionPathsCache[apiVersion] = paths } - if len(sensitiveFieldPaths) > 0 { - schema.RedactFields(resource.Properties, sensitiveFieldPaths) + fieldPaths := paths.forState(resource.ProvisioningState()) + if len(fieldPaths) > 0 { + schema.RedactFields(resource.Properties, fieldPaths) } } @@ -124,14 +124,14 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri } // Log redaction summary if any schemas were fetched - if len(sensitiveFieldPathsCache) > 0 { + if len(redactionPathsCache) > 0 { totalSensitiveFields := 0 - for _, paths := range sensitiveFieldPathsCache { - totalSensitiveFields += len(paths) + for _, paths := range redactionPathsCache { + totalSensitiveFields += len(paths.sensitive) } - logger.V(ucplog.LevelDebug).Info("Redacted sensitive fields in LIST response", + logger.V(ucplog.LevelDebug).Info("Redacted fields in LIST response", "totalSensitiveFields", totalSensitiveFields, - "apiVersions", len(sensitiveFieldPathsCache), + "apiVersions", len(redactionPathsCache), "resourceType", serviceCtx.ResourceID.Type(), "itemCount", len(items)) } diff --git a/pkg/dynamicrp/frontend/listresources_test.go b/pkg/dynamicrp/frontend/listresources_test.go index 8442ad96f21..8c7475f74db 100644 --- a/pkg/dynamicrp/frontend/listresources_test.go +++ b/pkg/dynamicrp/frontend/listresources_test.go @@ -119,7 +119,8 @@ func TestListResourcesWithRedaction_SucceededResourcesNotRedacted(t *testing.T) Items: []database.Object{*rpctest.FakeStoreObject(resource)}, }, nil) - // Schema should NOT be fetched for Succeeded resources + // A sensitive-only field (no x-radius-retain) is not redacted at Succeeded: the backend already + // redacted it to nil at rest. The schema is still fetched to look for retain fields. ucpClient, err := testUCPClientFactoryWithSensitiveFields() require.NoError(t, err) @@ -142,6 +143,62 @@ func TestListResourcesWithRedaction_SucceededResourcesNotRedacted(t *testing.T) require.Len(t, paginatedList.Value, 1) } +func TestListResourcesWithRedaction_SucceededRedactsRetainOnly(t *testing.T) { + // At Succeeded a retain field still holds ciphertext and must be redacted on read, while a + // sensitive-only field (already nil at rest) is returned as-is. This proves LIST no longer blanket- + // skips redaction for Succeeded items. + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newTestDynamicResource( + testResourceID, + "myResource", + v1.ProvisioningStateSucceeded, + map[string]any{ + "name": "test", + "password": "still-encrypted", + "apikey": "already-nil-at-rest", + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{*rpctest.FakeStoreObject(resource)}, + }, nil) + + ucpClient, err := testUCPClientFactoryWithRetainFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 1) + + // Inspect the returned item's properties via a JSON round-trip. + itemBytes, err := json.Marshal(paginatedList.Value[0]) + require.NoError(t, err) + var item map[string]any + require.NoError(t, json.Unmarshal(itemBytes, &item)) + properties, ok := item["properties"].(map[string]any) + require.True(t, ok) + require.Nil(t, properties["password"]) + require.Equal(t, "already-nil-at-rest", properties["apikey"]) +} + func TestListResourcesWithRedaction_NonSucceededResourcesRedacted(t *testing.T) { mctrl := gomock.NewController(t) defer mctrl.Finish() diff --git a/pkg/dynamicrp/frontend/redaction.go b/pkg/dynamicrp/frontend/redaction.go new file mode 100644 index 00000000000..b6915017b4f --- /dev/null +++ b/pkg/dynamicrp/frontend/redaction.go @@ -0,0 +1,67 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package frontend + +import ( + "context" + + v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + "github.com/radius-project/radius/pkg/schema" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" +) + +// redactionPaths holds the field paths a resource schema marks for redaction on read. +// +// - sensitive: every field marked x-radius-sensitive. +// - retain: the subset of sensitive fields also marked x-radius-retain. Retain fields keep their +// encrypted value at rest (vault semantics) instead of being redacted to nil by the backend, so +// they are the only sensitive fields still populated once a resource reaches Succeeded and MUST +// be redacted on read so the API never returns the retained ciphertext. +type redactionPaths struct { + sensitive []string + retain []string +} + +// fetchRedactionPaths fetches the schema for a resource type/api-version once and extracts both the +// sensitive and retain field paths from it. Returning empty paths (with no error) when the schema is +// unavailable mirrors GetSensitiveFieldPaths. +func fetchRedactionPaths(ctx context.Context, ucpClient *v20231001preview.ClientFactory, resourceID string, resourceType string, apiVersion string) (redactionPaths, error) { + schemaMap, err := schema.GetSchema(ctx, ucpClient, resourceID, resourceType, apiVersion) + if err != nil { + return redactionPaths{}, err + } + if schemaMap == nil { + return redactionPaths{}, nil + } + + return redactionPaths{ + sensitive: schema.ExtractSensitiveFieldPaths(schemaMap, ""), + retain: schema.ExtractRetainFieldPaths(schemaMap, ""), + }, nil +} + +// forState returns the field paths to redact for a resource in the given provisioning state. +// +// At Succeeded the backend has already redacted every non-retain sensitive field to nil, so only the +// retain fields (now holding ciphertext) remain to be redacted. In any other state the resource may +// still carry encrypted values for any sensitive field, so all sensitive fields are redacted. +func (p redactionPaths) forState(state v1.ProvisioningState) []string { + if state == v1.ProvisioningStateSucceeded { + return p.retain + } + return p.sensitive +} diff --git a/pkg/dynamicrp/options.go b/pkg/dynamicrp/options.go index ca4976f7d8e..8b176a8063a 100644 --- a/pkg/dynamicrp/options.go +++ b/pkg/dynamicrp/options.go @@ -29,6 +29,7 @@ import ( "github.com/radius-project/radius/pkg/components/kubernetesclient/kubernetesclientprovider" "github.com/radius-project/radius/pkg/components/queue/queueprovider" "github.com/radius-project/radius/pkg/components/secret/secretprovider" + "github.com/radius-project/radius/pkg/dynamicrp/secretsloader" "github.com/radius-project/radius/pkg/portableresources/processors" "github.com/radius-project/radius/pkg/recipes" "github.com/radius-project/radius/pkg/recipes/configloader" @@ -38,6 +39,7 @@ import ( "github.com/radius-project/radius/pkg/recipes/engine" "github.com/radius-project/radius/pkg/sdk" "github.com/radius-project/radius/pkg/sdk/clients" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" ucpconfig "github.com/radius-project/radius/pkg/ucp/config" sdk_cred "github.com/radius-project/radius/pkg/ucp/credentials" ) @@ -119,7 +121,17 @@ func NewOptions(ctx context.Context, config *Config) (*Options, error) { } options.Recipes.ConfigurationLoader = configloader.NewEnvironmentLoader(sdk.NewClientOptions(options.UCP)) - options.Recipes.SecretsLoader = configloader.NewSecretStoreLoader(sdk.NewClientOptions(options.UCP)) + + // The dispatching loader resolves Radius.Security/secrets resources by decrypting the value retained + // (encrypted) in the Radius store using the control-plane key, which is multi-cluster safe. It falls + // back to reading the backing Kubernetes Secret for secrets created before retain landed, and delegates + // other secret store types to the default Applications.Core/secretStores loader. + ucpClient, err := v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, sdk.NewClientOptions(options.UCP)) + if err != nil { + return nil, fmt.Errorf("failed to create UCP client for secrets loader: %w", err) + } + storeLoader := configloader.NewSecretStoreLoader(sdk.NewClientOptions(options.UCP)) + options.Recipes.SecretsLoader = secretsloader.NewDispatchingLoader(storeLoader, databaseClient, options.KubernetesProvider, ucpClient) // If this is set to nil, then the service will use the default recipe drivers. // diff --git a/pkg/dynamicrp/secretsloader/secretsloader.go b/pkg/dynamicrp/secretsloader/secretsloader.go new file mode 100644 index 00000000000..44ed44ad4c1 --- /dev/null +++ b/pkg/dynamicrp/secretsloader/secretsloader.go @@ -0,0 +1,250 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package secretsloader provides a configloader.SecretsLoader that can read Radius.Security/secrets +// resources in addition to the Applications.Core/secretStores resources handled by the default loader. +// +// A Radius.Security/secrets value is retained encrypted at rest (x-radius-retain): the backend keeps the +// frontend-encrypted value in the Radius store instead of redacting it to nil after recipe execution. This +// loader therefore resolves cleartext by decrypting the stored resource with the control-plane key (in +// radius-system), which works regardless of which cluster the application's Kubernetes Secret lives in +// (multi-cluster safe). +// +// The loader fails closed: if a secret's value is not retained encrypted at rest — for example a secret +// created before retain-at-rest landed, whose stored value is nil — it returns an error directing the +// operator to redeploy the secret, rather than silently falling back to reading the secret from a single +// cluster's Kubernetes Secret. +package secretsloader + +import ( + "context" + "fmt" + "strings" + + "github.com/radius-project/radius/pkg/components/database" + "github.com/radius-project/radius/pkg/components/kubernetesclient/kubernetesclientprovider" + "github.com/radius-project/radius/pkg/crypto/encryption" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel" + "github.com/radius-project/radius/pkg/recipes" + "github.com/radius-project/radius/pkg/recipes/configloader" + "github.com/radius-project/radius/pkg/schema" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/radius-project/radius/pkg/ucp/resources" +) + +const ( + // secretResourceType is the resource type of the user-defined secret resource. + secretResourceType = "Radius.Security/secrets" +) + +var _ configloader.SecretsLoader = (*dispatchingLoader)(nil) +var _ configloader.SecretsLoader = (*udtSecretsLoader)(nil) + +// NewDispatchingLoader returns a configloader.SecretsLoader that routes each secret ID to the appropriate +// backing loader based on its resource type: Radius.Security/secrets is resolved by decrypting the retained +// value from the Radius store, and all other types (e.g. Applications.Core/secretStores) are delegated to +// storeLoader. +func NewDispatchingLoader(storeLoader configloader.SecretsLoader, databaseClient database.Client, kubeProvider *kubernetesclientprovider.KubernetesClientProvider, ucpClient *v20231001preview.ClientFactory) configloader.SecretsLoader { + return &dispatchingLoader{ + storeLoader: storeLoader, + udtLoader: &udtSecretsLoader{databaseClient: databaseClient, kubeProvider: kubeProvider, ucpClient: ucpClient}, + } +} + +// dispatchingLoader routes secret IDs to a type-specific loader. +type dispatchingLoader struct { + storeLoader configloader.SecretsLoader + udtLoader configloader.SecretsLoader +} + +// LoadSecrets partitions the requested secret IDs by resource type and delegates each partition to the +// loader that can read it, then merges the results. +func (l *dispatchingLoader) LoadSecrets(ctx context.Context, secretStoreIDs map[string][]string) (map[string]recipes.SecretData, error) { + udtFilter := map[string][]string{} + storeFilter := map[string][]string{} + for id, keys := range secretStoreIDs { + parsed, err := resources.ParseResource(id) + if err != nil { + return nil, fmt.Errorf("failed to parse secret resource ID %q: %w", id, err) + } + + if strings.EqualFold(parsed.Type(), secretResourceType) { + udtFilter[id] = keys + } else { + storeFilter[id] = keys + } + } + + result := map[string]recipes.SecretData{} + + if len(udtFilter) > 0 { + loaded, err := l.udtLoader.LoadSecrets(ctx, udtFilter) + if err != nil { + return nil, err + } + for id, data := range loaded { + result[id] = data + } + } + + if len(storeFilter) > 0 { + if l.storeLoader == nil { + return nil, fmt.Errorf("no secret store loader is configured to load secrets from %d secret store(s)", len(storeFilter)) + } + loaded, err := l.storeLoader.LoadSecrets(ctx, storeFilter) + if err != nil { + return nil, err + } + for id, data := range loaded { + result[id] = data + } + } + + return result, nil +} + +// udtSecretsLoader reads cleartext for Radius.Security/secrets resources by decrypting the value retained +// encrypted in the Radius store with the control-plane key. +type udtSecretsLoader struct { + databaseClient database.Client + kubeProvider *kubernetesclientprovider.KubernetesClientProvider + ucpClient *v20231001preview.ClientFactory +} + +// LoadSecrets resolves each requested Radius.Security/secrets resource by decrypting its retained value from +// the Radius store and returns the cleartext data. +func (l *udtSecretsLoader) LoadSecrets(ctx context.Context, secretStoreIDs map[string][]string) (map[string]recipes.SecretData, error) { + result := map[string]recipes.SecretData{} + for id, keys := range secretStoreIDs { + data, err := l.loadSecret(ctx, id, keys) + if err != nil { + return nil, err + } + result[id] = data + } + + return result, nil +} + +// loadSecret resolves a single Radius.Security/secrets resource to its cleartext data by decrypting the +// value retained encrypted in the Radius store with the control-plane key. It fails closed: if the secret +// cannot be resolved — including a secret created before retain-at-rest landed, whose value is nil at rest — +// an error is returned rather than partial/empty data or a silent fallback to a single-cluster read. +func (l *udtSecretsLoader) loadSecret(ctx context.Context, secretID string, keysFilter []string) (recipes.SecretData, error) { + if l.databaseClient == nil || l.kubeProvider == nil { + return recipes.SecretData{}, fmt.Errorf("secret loader is not fully configured for Radius.Security/secrets") + } + + resource, err := database.GetResource[datamodel.DynamicResource](ctx, l.databaseClient, secretID) + if err != nil { + return recipes.SecretData{}, fmt.Errorf("failed to get secret resource %q: %w", secretID, err) + } + + return l.loadSecretFromStore(ctx, secretID, resource, keysFilter) +} + +// loadSecretFromStore decrypts the secret resource's retained value using the control-plane encryption key +// (held in radius-system) and returns its cleartext. Because the key lives on the control-plane cluster, +// decryption never depends on the application's target cluster, so it is multi-cluster safe. It fails closed +// if the value is not retained encrypted at rest. +func (l *udtSecretsLoader) loadSecretFromStore(ctx context.Context, secretID string, resource *datamodel.DynamicResource, keysFilter []string) (recipes.SecretData, error) { + if resource.Properties == nil { + return recipes.SecretData{}, fmt.Errorf("secret %q has no properties to resolve; redeploy the secret so its value is stored encrypted at rest", secretID) + } + + apiVersion := resource.InternalMetadata.UpdatedAPIVersion + schemaMap, err := schema.GetSchema(ctx, l.ucpClient, secretID, secretResourceType, apiVersion) + if err != nil { + return recipes.SecretData{}, fmt.Errorf("failed to fetch schema for secret %q: %w", secretID, err) + } + if schemaMap == nil { + return recipes.SecretData{}, fmt.Errorf("no schema is available for secret %q (%s, api-version %q); cannot decrypt its retained value", secretID, secretResourceType, apiVersion) + } + + sensitivePaths := schema.ExtractSensitiveFieldPaths(schemaMap, "") + + runtimeClient, err := l.kubeProvider.RuntimeClient() + if err != nil { + return recipes.SecretData{}, fmt.Errorf("failed to create Kubernetes client to decrypt secret %q: %w", secretID, err) + } + + // The encryption key lives in radius-system on the control-plane cluster, so decryption never depends + // on the target cluster the application is deployed to. + keyProvider := encryption.NewKubernetesKeyProvider(runtimeClient, nil) + handler, err := encryption.NewSensitiveDataHandlerFromProvider(ctx, keyProvider) + if err != nil { + return recipes.SecretData{}, fmt.Errorf("failed to create decryption handler for secret %q: %w", secretID, err) + } + + if err := handler.DecryptSensitiveFieldsWithSchema(ctx, resource.Properties, sensitivePaths, secretID, schemaMap); err != nil { + return recipes.SecretData{}, fmt.Errorf("failed to decrypt secret %q: %w", secretID, err) + } + + return buildSecretDataFromStore(secretID, resource.Properties, keysFilter) +} + +// buildSecretDataFromStore converts a decrypted Radius.Security/secrets properties map into recipes.SecretData. +// The secret's data property is a map of key to {value, encoding}; the (already decrypted) value is returned +// as-is, matching how Applications.Core/secretStores secrets are surfaced to recipes. +// +// When keysFilter is empty, all keys are returned; otherwise only the requested keys are returned. It fails +// closed: a missing requested key, or a value that is not retained at rest (nil, e.g. a secret created before +// retain landed), is an error rather than empty/incorrect data. +func buildSecretDataFromStore(secretID string, properties map[string]any, keysFilter []string) (recipes.SecretData, error) { + rawData, ok := properties["data"].(map[string]any) + if !ok { + return recipes.SecretData{}, fmt.Errorf("secret %q has no data stored at rest; redeploy the secret so its value is stored encrypted at rest", secretID) + } + + keys := keysFilter + if len(keys) == 0 { + keys = make([]string, 0, len(rawData)) + for key := range rawData { + keys = append(keys, key) + } + } + + result := recipes.SecretData{ + Type: secretResourceType, + Data: map[string]string{}, + } + + for _, key := range keys { + entryRaw, exists := rawData[key] + if !exists { + return recipes.SecretData{}, fmt.Errorf("secret key %q was not found in secret %q", key, secretID) + } + + entry, ok := entryRaw.(map[string]any) + if !ok { + return recipes.SecretData{}, fmt.Errorf("secret %q key %q has an unexpected format", secretID, key) + } + + value, exists := entry["value"] + if !exists || value == nil { + return recipes.SecretData{}, fmt.Errorf("secret %q key %q has no value stored at rest; it may have been created before secrets were retained encrypted at rest — redeploy the secret to populate its encrypted value", secretID, key) + } + + valueStr, ok := value.(string) + if !ok { + return recipes.SecretData{}, fmt.Errorf("secret %q key %q value is not a string", secretID, key) + } + + result.Data[key] = valueStr + } + + return result, nil +} diff --git a/pkg/dynamicrp/secretsloader/secretsloader_test.go b/pkg/dynamicrp/secretsloader/secretsloader_test.go new file mode 100644 index 00000000000..3f2828af8bc --- /dev/null +++ b/pkg/dynamicrp/secretsloader/secretsloader_test.go @@ -0,0 +1,425 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package secretsloader + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "net/http" + "testing" + + armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" + azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + "github.com/radius-project/radius/pkg/armrpc/rpctest" + aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" + "github.com/radius-project/radius/pkg/components/database" + "github.com/radius-project/radius/pkg/components/kubernetesclient/kubernetesclientprovider" + "github.com/radius-project/radius/pkg/crypto/encryption" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel" + "github.com/radius-project/radius/pkg/recipes" + "github.com/radius-project/radius/pkg/recipes/configloader" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + ucpfake "github.com/radius-project/radius/pkg/ucp/api/v20231001preview/fake" +) + +const ( + testSecretID = "/planes/radius/local/resourceGroups/test-rg/providers/Radius.Security/secrets/db-secret" + testStoreID = "/planes/radius/local/resourceGroups/test-rg/providers/Applications.Core/secretStores/store" + testInvalidID = "not-a-valid-id" +) + +// newKubeProvider builds a Kubernetes client provider backed by a fake runtime client holding the given objects. +func newKubeProvider(objects ...*corev1.Secret) *kubernetesclientprovider.KubernetesClientProvider { + builder := fake.NewClientBuilder() + for _, object := range objects { + builder = builder.WithObjects(object) + } + + provider := kubernetesclientprovider.FromConfig(nil) + provider.SetRuntimeClient(builder.Build()) + return provider +} + +func Test_DispatchingLoader_RadiusSecuritySecrets(t *testing.T) { + t.Run("fails closed when the resource cannot be read from the database", func(t *testing.T) { + ctrl := gomock.NewController(t) + databaseClient := database.NewMockClient(ctrl) + databaseClient.EXPECT().Get(gomock.Any(), testSecretID).Return(nil, errors.New("boom")) + + loader := NewDispatchingLoader(nil, databaseClient, newKubeProvider(), nil) + + _, err := loader.LoadSecrets(context.Background(), map[string][]string{testSecretID: nil}) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to get secret resource") + }) + + t.Run("fails closed when not configured", func(t *testing.T) { + loader := NewDispatchingLoader(nil, nil, nil, nil) + + _, err := loader.LoadSecrets(context.Background(), map[string][]string{testSecretID: nil}) + require.Error(t, err) + require.Contains(t, err.Error(), "not fully configured") + }) +} + +func Test_DispatchingLoader_Routing(t *testing.T) { + t.Run("delegates non-UDT secret types to the store loader", func(t *testing.T) { + ctrl := gomock.NewController(t) + storeLoader := configloader.NewMockSecretsLoader(ctrl) + storeLoader.EXPECT(). + LoadSecrets(gomock.Any(), map[string][]string{testStoreID: nil}). + Return(map[string]recipes.SecretData{ + testStoreID: {Type: "generic", Data: map[string]string{"key": "value"}}, + }, nil) + + loader := NewDispatchingLoader(storeLoader, nil, nil, nil) + + result, err := loader.LoadSecrets(context.Background(), map[string][]string{testStoreID: nil}) + require.NoError(t, err) + require.Equal(t, map[string]string{"key": "value"}, result[testStoreID].Data) + }) + + t.Run("routes each type to its loader and merges the results", func(t *testing.T) { + ctrl := gomock.NewController(t) + key, err := encryption.GenerateKey() + require.NoError(t, err) + + databaseClient := database.NewMockClient(ctrl) + databaseClient.EXPECT().Get(gomock.Any(), testSecretID).Return( + newEncryptedSecretObject(t, testSecretID, key, map[string]any{"password": "s3cr3t"}), nil) + kubeProvider := newKubeProvider(newEncryptionKeySecret(t, key)) + + storeLoader := configloader.NewMockSecretsLoader(ctrl) + storeLoader.EXPECT(). + LoadSecrets(gomock.Any(), map[string][]string{testStoreID: nil}). + Return(map[string]recipes.SecretData{ + testStoreID: {Type: "generic", Data: map[string]string{"key": "value"}}, + }, nil) + + loader := NewDispatchingLoader(storeLoader, databaseClient, kubeProvider, testSecretsUCPClientFactory(t, testSecretsSchema())) + + result, err := loader.LoadSecrets(context.Background(), map[string][]string{ + testSecretID: nil, + testStoreID: nil, + }) + require.NoError(t, err) + require.Equal(t, map[string]string{"password": "s3cr3t"}, result[testSecretID].Data) + require.Equal(t, map[string]string{"key": "value"}, result[testStoreID].Data) + }) + + t.Run("errors when a non-UDT secret is requested but no store loader is configured", func(t *testing.T) { + loader := NewDispatchingLoader(nil, nil, nil, nil) + + _, err := loader.LoadSecrets(context.Background(), map[string][]string{testStoreID: nil}) + require.Error(t, err) + require.Contains(t, err.Error(), "no secret store loader is configured") + }) + + t.Run("errors on an unparseable secret ID", func(t *testing.T) { + loader := NewDispatchingLoader(nil, nil, nil, nil) + + _, err := loader.LoadSecrets(context.Background(), map[string][]string{testInvalidID: nil}) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to parse secret resource ID") + }) +} + +const testSecretAPIVersion = "2023-10-01-preview" + +// Test_DispatchingLoader_RadiusSecuritySecrets_FromStore covers the primary (multi-cluster safe) path: +// resolving a secret by decrypting the value retained in the Radius store with the control-plane key. +func Test_DispatchingLoader_RadiusSecuritySecrets_FromStore(t *testing.T) { + t.Run("decrypts the retained value from the store", func(t *testing.T) { + ctrl := gomock.NewController(t) + key, err := encryption.GenerateKey() + require.NoError(t, err) + + databaseClient := database.NewMockClient(ctrl) + databaseClient.EXPECT().Get(gomock.Any(), testSecretID).Return( + newEncryptedSecretObject(t, testSecretID, key, map[string]any{"password": "s3cr3t", "username": "admin"}), nil) + + loader := NewDispatchingLoader(nil, databaseClient, newKubeProvider(newEncryptionKeySecret(t, key)), testSecretsUCPClientFactory(t, testSecretsSchema())) + + result, err := loader.LoadSecrets(context.Background(), map[string][]string{testSecretID: nil}) + require.NoError(t, err) + require.Equal(t, "Radius.Security/secrets", result[testSecretID].Type) + require.Equal(t, map[string]string{"password": "s3cr3t", "username": "admin"}, result[testSecretID].Data) + }) + + t.Run("returns only the requested keys from the store", func(t *testing.T) { + ctrl := gomock.NewController(t) + key, err := encryption.GenerateKey() + require.NoError(t, err) + + databaseClient := database.NewMockClient(ctrl) + databaseClient.EXPECT().Get(gomock.Any(), testSecretID).Return( + newEncryptedSecretObject(t, testSecretID, key, map[string]any{"password": "s3cr3t", "username": "admin"}), nil) + + loader := NewDispatchingLoader(nil, databaseClient, newKubeProvider(newEncryptionKeySecret(t, key)), testSecretsUCPClientFactory(t, testSecretsSchema())) + + result, err := loader.LoadSecrets(context.Background(), map[string][]string{testSecretID: {"password"}}) + require.NoError(t, err) + require.Equal(t, map[string]string{"password": "s3cr3t"}, result[testSecretID].Data) + }) + + t.Run("fails closed when a requested key is absent from the store", func(t *testing.T) { + ctrl := gomock.NewController(t) + key, err := encryption.GenerateKey() + require.NoError(t, err) + + databaseClient := database.NewMockClient(ctrl) + databaseClient.EXPECT().Get(gomock.Any(), testSecretID).Return( + newEncryptedSecretObject(t, testSecretID, key, map[string]any{"password": "s3cr3t"}), nil) + + loader := NewDispatchingLoader(nil, databaseClient, newKubeProvider(newEncryptionKeySecret(t, key)), testSecretsUCPClientFactory(t, testSecretsSchema())) + + _, err = loader.LoadSecrets(context.Background(), map[string][]string{testSecretID: {"missing"}}) + require.Error(t, err) + require.Contains(t, err.Error(), "was not found") + }) + + t.Run("fails closed when the value is nil at rest (pre-retain secret)", func(t *testing.T) { + ctrl := gomock.NewController(t) + key, err := encryption.GenerateKey() + require.NoError(t, err) + + // A secret created before retain-at-rest landed has a nil value in the store. Rather than silently + // falling back to a single-cluster Kubernetes read, the loader must fail closed and direct the + // operator to redeploy the secret. + databaseClient := database.NewMockClient(ctrl) + databaseClient.EXPECT().Get(gomock.Any(), testSecretID).Return( + newEncryptedSecretObject(t, testSecretID, key, map[string]any{"password": nil}), nil) + + loader := NewDispatchingLoader(nil, databaseClient, newKubeProvider(newEncryptionKeySecret(t, key)), testSecretsUCPClientFactory(t, testSecretsSchema())) + + _, err = loader.LoadSecrets(context.Background(), map[string][]string{testSecretID: nil}) + require.Error(t, err) + require.Contains(t, err.Error(), "redeploy the secret") + }) + + t.Run("fails closed when no schema is available to decrypt", func(t *testing.T) { + ctrl := gomock.NewController(t) + key, err := encryption.GenerateKey() + require.NoError(t, err) + + // With no ucpClient the schema cannot be fetched, so the retained value cannot be decrypted. The + // loader must fail closed rather than silently returning empty data. + databaseClient := database.NewMockClient(ctrl) + databaseClient.EXPECT().Get(gomock.Any(), testSecretID).Return( + newEncryptedSecretObject(t, testSecretID, key, map[string]any{"password": "s3cr3t"}), nil) + + loader := NewDispatchingLoader(nil, databaseClient, newKubeProvider(newEncryptionKeySecret(t, key)), nil) + + _, err = loader.LoadSecrets(context.Background(), map[string][]string{testSecretID: nil}) + require.Error(t, err) + require.Contains(t, err.Error(), "no schema is available") + }) +} + +func Test_buildSecretDataFromStore(t *testing.T) { + const id = "/planes/radius/local/resourceGroups/test-rg/providers/Radius.Security/secrets/s" + + t.Run("returns all keys when no filter is given", func(t *testing.T) { + props := map[string]any{"data": map[string]any{ + "a": map[string]any{"value": "1"}, + "b": map[string]any{"value": "2"}, + }} + data, err := buildSecretDataFromStore(id, props, nil) + require.NoError(t, err) + require.Equal(t, map[string]string{"a": "1", "b": "2"}, data.Data) + require.Equal(t, secretResourceType, data.Type) + }) + + t.Run("returns only the requested keys", func(t *testing.T) { + props := map[string]any{"data": map[string]any{ + "a": map[string]any{"value": "1"}, + "b": map[string]any{"value": "2"}, + }} + data, err := buildSecretDataFromStore(id, props, []string{"a"}) + require.NoError(t, err) + require.Equal(t, map[string]string{"a": "1"}, data.Data) + }) + + t.Run("a missing requested key is an error", func(t *testing.T) { + props := map[string]any{"data": map[string]any{"a": map[string]any{"value": "1"}}} + _, err := buildSecretDataFromStore(id, props, []string{"c"}) + require.Error(t, err) + require.Contains(t, err.Error(), "was not found") + }) + + t.Run("a nil value fails closed (pre-retain secret)", func(t *testing.T) { + props := map[string]any{"data": map[string]any{"a": map[string]any{"value": nil}}} + _, err := buildSecretDataFromStore(id, props, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "redeploy the secret") + }) + + t.Run("an absent data property fails closed", func(t *testing.T) { + _, err := buildSecretDataFromStore(id, map[string]any{}, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "no data stored at rest") + }) + + t.Run("a non-string value is an error", func(t *testing.T) { + props := map[string]any{"data": map[string]any{"a": map[string]any{"value": 123}}} + _, err := buildSecretDataFromStore(id, props, []string{"a"}) + require.Error(t, err) + require.Contains(t, err.Error(), "is not a string") + }) + + t.Run("an unexpected entry format is an error", func(t *testing.T) { + props := map[string]any{"data": map[string]any{"a": "not-a-map"}} + _, err := buildSecretDataFromStore(id, props, []string{"a"}) + require.Error(t, err) + require.Contains(t, err.Error(), "unexpected format") + }) +} + +// testSecretsSchema returns a minimal Radius.Security/secrets schema whose data[*].value field is marked +// sensitive, matching the production schema shape used to extract sensitive field paths. +func testSecretsSchema() map[string]any { + return map[string]any{ + "properties": map[string]any{ + "data": map[string]any{ + "additionalProperties": map[string]any{ + "properties": map[string]any{ + "value": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }, + }, + }, + } +} + +// testSecretsUCPClientFactory builds a v20231001preview.ClientFactory backed by a fake transport that +// returns the given schema for any API version lookup. +func testSecretsUCPClientFactory(t *testing.T, schema map[string]any) *v20231001preview.ClientFactory { + t.Helper() + + apiVersionsServer := ucpfake.APIVersionsServer{ + Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (resp azfake.Responder[v20231001preview.APIVersionsClientGetResponse], errResp azfake.ErrorResponder) { + response := v20231001preview.APIVersionsClientGetResponse{ + APIVersionResource: v20231001preview.APIVersionResource{ + Properties: &v20231001preview.APIVersionProperties{ + Schema: schema, + }, + }, + } + resp.SetResponse(http.StatusOK, response, nil) + return + }, + } + + factory, err := v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Transport: ucpfake.NewAPIVersionsServerTransport(&apiVersionsServer), + }, + }) + require.NoError(t, err) + return factory +} + +// newEncryptionKeySecret builds the radius-system Kubernetes Secret that holds the versioned encryption key +// store the loader uses to decrypt retained secret values. +func newEncryptionKeySecret(t *testing.T, key []byte) *corev1.Secret { + t.Helper() + + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: encryption.DefaultEncryptionKeySecretName, + Namespace: encryption.RadiusNamespace, + }, + Data: map[string][]byte{ + encryption.DefaultEncryptionKeySecretKey: mustKeyStoreJSON(t, key), + }, + } +} + +// newEncryptedSecretObject builds a database.Object for a Radius.Security/secrets resource whose data +// property holds values encrypted with the given key, mirroring how the frontend encrypts and the backend +// retains them at rest. A nil entry value represents a pre-retain (redacted) secret, which the loader must +// fail closed on rather than silently resolve. +func newEncryptedSecretObject(t *testing.T, resourceID string, key []byte, values map[string]any) *database.Object { + t.Helper() + + provider, err := encryption.NewInMemoryKeyProvider(key) + require.NoError(t, err) + handler, err := encryption.NewSensitiveDataHandlerFromProvider(context.Background(), provider) + require.NoError(t, err) + + data := map[string]any{} + for name, value := range values { + data[name] = map[string]any{"value": value} + } + + properties := map[string]any{ + "provisioningState": "Succeeded", + "data": data, + } + + require.NoError(t, handler.EncryptSensitiveFields(properties, []string{"data[*].value"}, resourceID)) + + resource := &datamodel.DynamicResource{ + BaseResource: v1.BaseResource{ + TrackedResource: v1.TrackedResource{ + ID: resourceID, + Name: "db-secret", + Type: "Radius.Security/secrets", + }, + InternalMetadata: v1.InternalMetadata{ + UpdatedAPIVersion: testSecretAPIVersion, + }, + }, + Properties: properties, + } + + return rpctest.FakeStoreObject(resource) +} + +// mustKeyStoreJSON serializes a single-version key store for the radius-system encryption key Secret. +func mustKeyStoreJSON(t *testing.T, key []byte) []byte { + t.Helper() + + keyStore := encryption.KeyStore{ + CurrentVersion: 1, + Keys: map[string]encryption.KeyData{ + "1": { + Key: base64.StdEncoding.EncodeToString(key), + Version: 1, + CreatedAt: "2026-01-01T00:00:00Z", + ExpiresAt: "2026-12-31T00:00:00Z", + }, + }, + } + + bytes, err := json.Marshal(keyStore) + require.NoError(t, err) + return bytes +} diff --git a/pkg/portableresources/backend/controller/createorupdateresource.go b/pkg/portableresources/backend/controller/createorupdateresource.go index e0ae4f06bde..5845a0f63ce 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource.go +++ b/pkg/portableresources/backend/controller/createorupdateresource.go @@ -18,9 +18,11 @@ package controller import ( "context" + "encoding/base64" "encoding/json" "errors" "fmt" + "strings" "github.com/go-logr/logr" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" @@ -36,9 +38,18 @@ import ( "github.com/radius-project/radius/pkg/resourceutil" rpv1 "github.com/radius-project/radius/pkg/rp/v1" schemautil "github.com/radius-project/radius/pkg/schema" + "github.com/radius-project/radius/pkg/ucp/resources" "github.com/radius-project/radius/pkg/ucp/ucplog" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) +// secretReferenceResourceType is the resource type of a Radius.Security/secrets resource named by an +// x-radius-secret-reference property. +const secretReferenceResourceType = "Radius.Security/secrets" + // CreateOrUpdateResource is the async operation controller to create or update portable resources. type CreateOrUpdateResource[P interface { *T @@ -82,6 +93,8 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques currentETag := storedResource.ETag var recipeProperties map[string]any + var secretReferencePaths []string + var secretBindingPaths []string redactionCompleted := false // Attempt to decrypt and redact sensitive fields before recipe execution. @@ -92,9 +105,18 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques if schemaErr != nil { return ctrl.Result{}, fmt.Errorf("failed to fetch schema for sensitive field detection: %w", schemaErr) } else if schema != nil { + // Detect properties that reference a Radius.Security/secrets resource by name so the engine can + // resolve them into recipe context (context.resource.secrets.). + secretReferencePaths = schemautil.ExtractSecretReferenceFieldPaths(schema, "") + + // Detect secrets arrays (x-radius-secret-binding) so the engine can load each listed + // secret's keys into recipe context (context.resource.secrets..). + secretBindingPaths = schemautil.ExtractSecretBindingPaths(schema, "") + sensitiveFieldPaths := schemautil.ExtractSensitiveFieldPaths(schema, "") + retainFieldPaths := schemautil.ExtractRetainFieldPaths(schema, "") if len(sensitiveFieldPaths) > 0 { - logger.Info("Sensitive fields detected for resource", "resourceID", req.ResourceID, "paths", sensitiveFieldPaths) + logger.Info("Sensitive fields detected for resource", "resourceID", req.ResourceID, "paths", sensitiveFieldPaths, "retain", retainFieldPaths) properties, err := resourceutil.GetPropertiesFromResource(resource) if err != nil { @@ -131,7 +153,12 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques if err != nil { return ctrl.Result{}, err } - schemautil.RedactFields(redactedProperties, sensitiveFieldPaths) + // Retain-marked fields (e.g. Radius.Security/secrets data values) keep their encrypted + // value at rest so the secrets loader can decrypt from the store instead of reading a + // cluster (multi-cluster safe). Only non-retain sensitive fields are redacted to nil here; + // retain fields are redacted on read (GET/LIST) instead. + redactFieldPaths := pathsExcept(sensitiveFieldPaths, retainFieldPaths) + schemautil.RedactFields(redactedProperties, redactFieldPaths) if err = applyPropertiesToResource(resource, redactedProperties); err != nil { logger.Error(err, "Failed to apply redacted properties", "resourceID", req.ResourceID) @@ -171,7 +198,7 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques recipeDataModel, supportsRecipes := any(resource).(datamodel.RecipeDataModel) var recipeOutput *recipes.RecipeOutput if supportsRecipes && recipeDataModel.GetRecipe() != nil { - recipeOutput, err = c.executeRecipeIfNeeded(ctx, resource, recipeDataModel, previousOutputResources, config.Simulated, recipeProperties) + recipeOutput, err = c.executeRecipeIfNeeded(ctx, resource, recipeDataModel, previousOutputResources, config.Simulated, recipeProperties, secretReferencePaths, secretBindingPaths) if err != nil { return c.handleRecipeError(ctx, err, recipeDataModel, req.ResourceID, currentETag, logger, redactionCompleted) } @@ -188,6 +215,20 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques } return ctrl.Result{}, err } + + // Apply any secret write-backs the recipe driver resolved: secure module outputs routed into + // developer-authored Radius.Security/secrets resources bound to this resource (the "backwards" + // secret flow). This patches each bound secret's retained (encrypted) value in the store and + // refreshes its backing Kubernetes Secret so connected containers can consume it via secretKeyRef. + if recipeOutput != nil && len(recipeOutput.SecretWriteBacks) > 0 { + if err = c.applySecretWriteBacks(ctx, recipeOutput.SecretWriteBacks, config.Runtime); err != nil { + logger.Error(err, "Failed to apply secret write-backs", "resourceID", req.ResourceID) + if redactionCompleted { + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } + return ctrl.Result{}, err + } + } } if supportsRecipes { @@ -262,7 +303,7 @@ func (c *CreateOrUpdateResource[P, T]) copyOutputResources(resource P) []string return previousOutputResources } -func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context, resource P, recipeDataModel datamodel.RecipeDataModel, prevState []string, simulated bool, recipeProperties map[string]any) (*recipes.RecipeOutput, error) { +func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context, resource P, recipeDataModel datamodel.RecipeDataModel, prevState []string, simulated bool, recipeProperties map[string]any, secretReferencePaths []string, secretBindingPaths []string) (*recipes.RecipeOutput, error) { // Caller ensures recipeDataModel supports recipes and has a non-nil recipe recipe := recipeDataModel.GetRecipe() @@ -295,6 +336,23 @@ func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context return nil, fmt.Errorf("failed to get metadata from connected resource %s: %w", connectedResourceID, err) } + // Redact sensitive fields from a connected Radius.Security/secrets resource before exposing its + // properties to the recipe. Its value is retained encrypted at rest (x-radius-retain), and must not + // be surfaced — even as ciphertext — through the recipe context; secrets are consumed via secret + // bindings/references, not connections. Other resource types still redact their sensitive fields to + // nil at rest, so they need no redaction here. + if strings.EqualFold(connectedResourceMetadata.Type, secretReferenceResourceType) && len(connectedResourceMetadata.Properties) > 0 { + if apiVersion := connectedResourceAPIVersion(connectedResource.Data); apiVersion != "" { + connectedSchema, err := schemautil.GetSchema(ctx, c.UcpClient(), connectedResourceID, connectedResourceMetadata.Type, apiVersion) + if err != nil { + return nil, fmt.Errorf("failed to fetch schema to redact connected secret %s: %w", connectedResourceID, err) + } + if connectedSchema != nil { + schemautil.RedactFields(connectedResourceMetadata.Properties, schemautil.ExtractSensitiveFieldPaths(connectedSchema, "")) + } + } + } + connectedResourcesMetadata[connName] = recipes.ConnectedResource{ ID: connectedResourceMetadata.ID, Name: connectedResourceMetadata.Name, @@ -313,6 +371,22 @@ func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context ConnectedResourcesProperties: connectedResourcesMetadata, } + // Resolve x-radius-secret-reference properties to Radius.Security/secrets resource IDs so the engine + // can load their secret material into the recipe context. + secretReferences, err := buildSecretReferences(resource.GetBaseResource().ID, resourceProperties, secretReferencePaths) + if err != nil { + return nil, err + } + metadata.SecretReferences = secretReferences + + // Resolve the x-radius-secret-binding array property to the list of secret IDs it references so the engine + // can load each bound secret's keys into the recipe context. + secretBindings, err := buildSecretBindings(resourceProperties, secretBindingPaths) + if err != nil { + return nil, err + } + metadata.SecretBindings = secretBindings + return c.engine.Execute(ctx, engine.ExecuteOptions{ BaseOptions: engine.BaseOptions{ Recipe: metadata, @@ -326,6 +400,348 @@ func getResourceAPIVersion[P rpv1.RadiusResourceModel](resource P) string { return resource.GetBaseResource().InternalMetadata.UpdatedAPIVersion } +// connectedResourceAPIVersion extracts the api-version a connected resource was last updated with from its +// raw stored representation, so the connected resource's schema can be fetched to redact its sensitive fields. +// Returns "" if the api-version cannot be determined. +func connectedResourceAPIVersion(data any) string { + var partial struct { + UpdatedAPIVersion string `json:"updatedApiVersion"` + } + + bs, err := json.Marshal(data) + if err != nil { + return "" + } + if err := json.Unmarshal(bs, &partial); err != nil { + return "" + } + + return partial.UpdatedAPIVersion +} + +// applySecretWriteBacks writes secure recipe (module) outputs back into the developer-authored +// Radius.Security/secrets resources bound to the deploying resource (the "backwards" secret flow). For each +// write-back it: (1) reads the bound secret from the store, (2) encrypts only the new values with the +// control-plane key — bound to the secret's own resource ID and sensitive field paths so it matches how the +// secret is encrypted at rest and the loader can later decrypt it — merges them into the secret's retained +// data, and saves it, and (3) refreshes the secret's backing Kubernetes Secret so connected containers +// consuming it via secretKeyRef observe the resolved value. It fails closed: any error aborts the deployment +// rather than leaving a container to read an empty secret. +// +// NOTE (single-cluster): the backing Kubernetes Secret is written through the control-plane Kubernetes +// client (the same cluster the secret's own recipe targets in the default single-cluster topology). +// Cross-cluster write-back of the backing Secret is out of scope; the encrypted-at-rest store copy is the +// multi-cluster source of truth for recipe reads. +func (c *CreateOrUpdateResource[P, T]) applySecretWriteBacks(ctx context.Context, writeBacks []recipes.SecretWriteBack, runtime recipes.RuntimeConfiguration) error { + logger := ucplog.FromContextOrDiscard(ctx) + + if c.KubeClient() == nil { + return fmt.Errorf("kubernetes client not configured for secret write-back") + } + keyProvider := encryption.NewKubernetesKeyProvider(c.KubeClient(), nil) + handler, err := encryption.NewSensitiveDataHandlerFromProvider(ctx, keyProvider) + if err != nil { + return fmt.Errorf("failed to initialize sensitive data handler for secret write-back: %w", err) + } + + if runtime.Kubernetes == nil || runtime.Kubernetes.Namespace == "" { + return fmt.Errorf("no Kubernetes namespace available to write back secrets") + } + namespace := runtime.Kubernetes.Namespace + + for _, wb := range writeBacks { + if len(wb.Data) == 0 { + continue + } + + obj, err := c.DatabaseClient().Get(ctx, wb.SecretID) + if err != nil { + return fmt.Errorf("failed to read bound secret %q for write-back: %w", wb.SecretID, err) + } + + var secretResource map[string]any + if err := obj.As(&secretResource); err != nil { + return fmt.Errorf("failed to decode bound secret %q for write-back: %w", wb.SecretID, err) + } + + apiVersion := connectedResourceAPIVersion(obj.Data) + secretSchema, err := schemautil.GetSchema(ctx, c.UcpClient(), wb.SecretID, secretReferenceResourceType, apiVersion) + if err != nil { + return fmt.Errorf("failed to fetch schema for bound secret %q: %w", wb.SecretID, err) + } + if secretSchema == nil { + return fmt.Errorf("no schema available for bound secret %q (%s, api-version %q); cannot write back its value", wb.SecretID, secretReferenceResourceType, apiVersion) + } + sensitiveFieldPaths := schemautil.ExtractSensitiveFieldPaths(secretSchema, "") + + properties, ok := secretResource["properties"].(map[string]any) + if !ok { + return fmt.Errorf("bound secret %q has no properties to write back into", wb.SecretID) + } + data, ok := properties["data"].(map[string]any) + if !ok || data == nil { + data = map[string]any{} + } + + // Encrypt ONLY the new values. The stored data is already encrypted at rest (retain), so + // re-encrypting it would double-encrypt. Build a minimal {data: {key: {value}}} carrying just the + // new values and encrypt it with the secret's own resource ID + sensitive paths (matching at-rest + // encryption so a later read can decrypt), then merge the ciphertext into the retained data. + encData := make(map[string]any, len(wb.Data)) + for key, value := range wb.Data { + encData[key] = map[string]any{"value": value} + } + encWrapper := map[string]any{"data": encData} + if err := handler.EncryptSensitiveFields(encWrapper, sensitiveFieldPaths, wb.SecretID); err != nil { + return fmt.Errorf("failed to encrypt write-back values for secret %q: %w", wb.SecretID, err) + } + encryptedData, _ := encWrapper["data"].(map[string]any) + + // Build the plaintext view keyed by encoding for the backing Kubernetes Secret, and merge the + // encrypted entries into the store's retained data (preserving each key's existing encoding). + stringData := map[string]string{} + base64Data := map[string]string{} + for key, plaintext := range wb.Data { + encoding := "string" + if existing, ok := data[key].(map[string]any); ok { + if enc, ok := existing["encoding"].(string); ok && enc != "" { + encoding = enc + } + } + + entry := map[string]any{"encoding": encoding} + if encEntry, ok := encryptedData[key].(map[string]any); ok { + entry["value"] = encEntry["value"] + } + data[key] = entry + + if encoding == "base64" { + base64Data[key] = plaintext + } else { + stringData[key] = plaintext + } + } + properties["data"] = data + secretResource["properties"] = properties + + update := &database.Object{ + Metadata: database.Metadata{ID: wb.SecretID}, + Data: secretResource, + } + if err := c.DatabaseClient().Save(ctx, update, database.WithETag(obj.ETag)); err != nil { + return fmt.Errorf("failed to persist write-back for secret %q: %w", wb.SecretID, err) + } + + if err := c.upsertBackingSecret(ctx, wb.SecretID, namespace, stringData, base64Data); err != nil { + return fmt.Errorf("failed to refresh backing Kubernetes Secret for secret %q: %w", wb.SecretID, err) + } + + logger.Info("Applied secret write-back", "secretID", wb.SecretID, "keyCount", len(wb.Data), "namespace", namespace) + } + + return nil +} + +// upsertBackingSecret refreshes the Kubernetes Secret that backs a Radius.Security/secrets resource so a +// container consuming it via secretKeyRef observes the written-back value. The Secret's name matches the +// secret resource's name (as materialized by the secrets recipe). Existing keys are preserved: string-encoded +// values are written to StringData and base64-encoded values to Data. If the Secret does not yet exist it is +// created (best effort) with the recipe's resource label. +func (c *CreateOrUpdateResource[P, T]) upsertBackingSecret(ctx context.Context, secretID string, namespace string, stringData map[string]string, base64Data map[string]string) error { + parsed, err := resources.ParseResource(secretID) + if err != nil { + return fmt.Errorf("failed to parse secret ID %q: %w", secretID, err) + } + name := parsed.Name() + + kubeClient := c.KubeClient() + secret := &corev1.Secret{} + getErr := kubeClient.Get(ctx, runtimeclient.ObjectKey{Namespace: namespace, Name: name}, secret) + if getErr != nil { + if !apierrors.IsNotFound(getErr) { + return fmt.Errorf("failed to read backing Kubernetes Secret %q in namespace %q: %w", name, namespace, getErr) + } + + // The secret's own recipe should have created this Secret already; create it as a fail-safe. + secret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{"resource": name}, + }, + Type: corev1.SecretTypeOpaque, + } + applyBackingSecretData(secret, stringData, base64Data) + if err := kubeClient.Create(ctx, secret); err != nil { + return fmt.Errorf("failed to create backing Kubernetes Secret %q in namespace %q: %w", name, namespace, err) + } + return nil + } + + applyBackingSecretData(secret, stringData, base64Data) + if err := kubeClient.Update(ctx, secret); err != nil { + return fmt.Errorf("failed to update backing Kubernetes Secret %q in namespace %q: %w", name, namespace, err) + } + return nil +} + +// applyBackingSecretData merges the written-back values into a Kubernetes Secret, preserving other keys. +// Both string- and base64-encoded values are written to Data as raw bytes (string values verbatim, base64 +// values decoded), so the result matches how the secrets recipe materializes the Secret and does not depend +// on the API server folding StringData into Data. +func applyBackingSecretData(secret *corev1.Secret, stringData map[string]string, base64Data map[string]string) { + if len(stringData) == 0 && len(base64Data) == 0 { + return + } + if secret.Data == nil { + secret.Data = map[string][]byte{} + } + for k, v := range stringData { + secret.Data[k] = []byte(v) + } + for k, v := range base64Data { + if decoded, err := base64.StdEncoding.DecodeString(v); err == nil { + secret.Data[k] = decoded + } else { + // Not valid base64; store the raw bytes so the value is still surfaced. + secret.Data[k] = []byte(v) + } + } +} + +// buildSecretReferences resolves x-radius-secret-reference property values (each the name of a +// Radius.Security/secrets resource) to fully qualified resource IDs scoped to the consuming resource's +// resource group. Unset reference properties are skipped; a malformed reference is an error so the +// deployment fails closed rather than passing an unresolved value to the recipe. +func buildSecretReferences(resourceID string, properties map[string]any, secretReferencePaths []string) (map[string]string, error) { + if len(secretReferencePaths) == 0 { + return nil, nil + } + + parsed, err := resources.ParseResource(resourceID) + if err != nil { + return nil, fmt.Errorf("failed to parse resource ID %q for secret reference resolution: %w", resourceID, err) + } + + references := map[string]string{} + for _, path := range secretReferencePaths { + name, ok := stringValueAtPath(properties, path) + if !ok || name == "" { + // The reference property is optional and unset; nothing to resolve. + continue + } + + secretID := fmt.Sprintf("%s/providers/%s/%s", parsed.RootScope(), secretReferenceResourceType, name) + if _, err := resources.ParseResource(secretID); err != nil { + return nil, fmt.Errorf("failed to construct secret resource ID for property %q with value %q: %w", path, name, err) + } + references[path] = secretID + } + + if len(references) == 0 { + return nil, nil + } + + return references, nil +} + +// stringValueAtPath returns the string value at the given dot-separated path in a nested properties map. +func stringValueAtPath(properties map[string]any, path string) (string, bool) { + var current any = properties + for _, segment := range strings.Split(path, ".") { + m, ok := current.(map[string]any) + if !ok { + return "", false + } + current, ok = m[segment] + if !ok { + return "", false + } + } + + value, ok := current.(string) + return value, ok +} + +// buildSecretBindings resolves an x-radius-secret-binding array property into the list of Radius.Security/secrets +// resource IDs it references. Each entry must be a non-empty string that parses as a resource ID; the engine +// loads every key of each secret into the recipe's Secrets under a "." namespace for the +// context.resource.secrets.. expression path. An unset property is skipped; a malformed entry +// (non-string, empty, or not a resource ID) is an error so the deployment fails closed rather than silently +// dropping a secret the recipe expects. +func buildSecretBindings(properties map[string]any, secretBindingPaths []string) ([]string, error) { + if len(secretBindingPaths) == 0 { + return nil, nil + } + + var secretIDs []string + for _, path := range secretBindingPaths { + raw, ok := valueAtPath(properties, path) + if !ok || raw == nil { + // The property is optional and unset; nothing to resolve. + continue + } + + entries, ok := raw.([]any) + if !ok { + return nil, fmt.Errorf("secret binding property %q must be an array of secret IDs", path) + } + + for i, entry := range entries { + id, ok := entry.(string) + if !ok || id == "" { + return nil, fmt.Errorf("secret binding %q[%d] must be a non-empty secret ID string", path, i) + } + if _, err := resources.ParseResource(id); err != nil { + return nil, fmt.Errorf("secret binding %q[%d] has invalid secret ID %q: %w", path, i, id, err) + } + secretIDs = append(secretIDs, id) + } + } + + if len(secretIDs) == 0 { + return nil, nil + } + + return secretIDs, nil +} + +// valueAtPath returns the value at the given dot-separated path in a nested properties map. +func valueAtPath(properties map[string]any, path string) (any, bool) { + var current any = properties + for _, segment := range strings.Split(path, ".") { + m, ok := current.(map[string]any) + if !ok { + return nil, false + } + current, ok = m[segment] + if !ok { + return nil, false + } + } + + return current, true +} + +// pathsExcept returns the elements of all that are not present in exclude. Used to redact only the +// sensitive field paths that are NOT retain-marked (retain fields keep their encrypted value at rest). +func pathsExcept(all []string, exclude []string) []string { + if len(exclude) == 0 { + return all + } + excluded := make(map[string]struct{}, len(exclude)) + for _, p := range exclude { + excluded[p] = struct{}{} + } + result := make([]string, 0, len(all)) + for _, p := range all { + if _, found := excluded[p]; !found { + result = append(result, p) + } + } + return result +} + func deepCopyProperties(source map[string]any) (map[string]any, error) { if source == nil { return map[string]any{}, nil diff --git a/pkg/portableresources/backend/controller/createorupdateresource_test.go b/pkg/portableresources/backend/controller/createorupdateresource_test.go index 201a0946075..b8142f1c59e 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource_test.go +++ b/pkg/portableresources/backend/controller/createorupdateresource_test.go @@ -35,6 +35,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" controllerfake "sigs.k8s.io/controller-runtime/pkg/client/fake" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" @@ -106,6 +107,7 @@ type TestResourceProperties struct { IsProcessed bool `json:"isProcessed"` Recipe portableresources.ResourceRecipe `json:"recipe"` Secret any `json:"secret,omitempty"` + Plain any `json:"plain,omitempty"` Credentials map[string]any `json:"credentials,omitempty"` } @@ -637,6 +639,163 @@ func TestCreateOrUpdateResource_Run_SensitiveRedaction(t *testing.T) { require.Equal(t, ctrl.Result{}, res) } +func TestCreateOrUpdateResource_Run_RetainEncrypted(t *testing.T) { + // A field marked x-radius-retain (in addition to x-radius-sensitive) must keep its encrypted value at + // rest instead of being redacted to nil, so the secrets loader can decrypt it from the store. A + // sensitive-only field on the same resource must still be redacted. In both cases the recipe must + // receive the decrypted cleartext. + mctrl := gomock.NewController(t) + msc := database.NewMockClient(mctrl) + eng := engine.NewMockEngine(mctrl) + cfg := configloader.NewMockConfigurationLoader(mctrl) + + key, err := encryption.GenerateKey() + require.NoError(t, err) + + provider, err := encryption.NewInMemoryKeyProvider(key) + require.NoError(t, err) + + handler, err := encryption.NewSensitiveDataHandlerFromProvider(context.Background(), provider) + require.NoError(t, err) + + retainValue := "retain-me" + plainValue := "redact-me" + properties := map[string]any{ + "application": TestApplicationID, + "environment": TestEnvironmentID, + "provisioningState": "Accepted", + "secret": retainValue, + "plain": plainValue, + "recipe": map[string]any{ + "name": "test-recipe", + }, + "status": map[string]any{ + "outputResources": []map[string]any{}, + }, + } + + err = handler.EncryptSensitiveFields(properties, []string{"secret", "plain"}, TestResourceID) + require.NoError(t, err) + + data := map[string]any{ + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "updatedApiVersion": "2024-01-01", + "properties": properties, + } + + msc.EXPECT(). + Get(gomock.Any(), TestResourceID). + Return(&database.Object{Metadata: database.Metadata{ID: TestResourceID, ETag: "etag-1"}, Data: data}, nil). + Times(1) + + ucpClient, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "secret": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + "x-radius-retain": true, + }, + "plain": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }) + require.NoError(t, err) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: encryption.DefaultEncryptionKeySecretName, + Namespace: encryption.RadiusNamespace, + }, + Data: map[string][]byte{ + encryption.DefaultEncryptionKeySecretKey: mustKeyStoreJSON(t, key), + }, + } + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + k8sClient := controllerfake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build() + + configuration := &recipes.Configuration{ + Runtime: recipes.RuntimeConfiguration{ + Kubernetes: &recipes.KubernetesRuntime{ + Namespace: "test-namespace", + EnvironmentNamespace: "test-env-namespace", + }, + }, + } + + // assertPersisted verifies the retain field keeps its encrypted envelope while the sensitive-only + // field is redacted to nil. + assertPersisted := func(obj *database.Object) { + props, err := resourceutil.GetPropertiesFromResource(obj.Data) + require.NoError(t, err) + require.Nil(t, props["plain"]) + retained, ok := props["secret"].(map[string]any) + require.True(t, ok, "retain field should keep its encrypted envelope, got %T", props["secret"]) + require.Contains(t, retained, "encrypted") + require.Contains(t, retained, "nonce") + } + + redactionSave := msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, obj *database.Object, _ ...database.SaveOptions) error { + assertPersisted(obj) + obj.Metadata.ETag = "etag-2" + return nil + }, + ) + + finalSave := msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, obj *database.Object, _ ...database.SaveOptions) error { + assertPersisted(obj) + return nil + }, + ) + + engExecute := eng.EXPECT().Execute(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, opts engine.ExecuteOptions) (*recipes.RecipeOutput, error) { + require.Equal(t, retainValue, opts.BaseOptions.Recipe.Properties["secret"]) + require.Equal(t, plainValue, opts.BaseOptions.Recipe.Properties["plain"]) + return &recipes.RecipeOutput{}, nil + }, + ) + + gomock.InOrder( + redactionSave, + cfg.EXPECT().LoadConfiguration(gomock.Any(), recipes.ResourceMetadata{EnvironmentID: TestEnvironmentID, ApplicationID: TestApplicationID, ResourceID: TestResourceID}).Return(configuration, nil), + engExecute, + finalSave, + ) + + opts := ctrl.Options{ + DatabaseClient: msc, + UcpClient: ucpClient, + KubeClient: k8sClient, + } + + recipeCfg := &controllerconfig.RecipeControllerConfig{ + Engine: eng, + ConfigLoader: cfg, + } + + ctrlr, err := NewCreateOrUpdateResource(opts, successProcessorReference, recipeCfg.Engine, recipeCfg.ConfigLoader) + require.NoError(t, err) + + res, err := ctrlr.Run(context.Background(), &ctrl.Request{ + OperationID: uuid.New(), + OperationType: "APPLICATIONS.TEST/TESTRESOURCES|PUT", + ResourceID: TestResourceID, + CorrelationID: uuid.NewString(), + OperationTimeout: &ctrl.DefaultAsyncOperationTimeout, + }) + require.NoError(t, err) + require.Equal(t, ctrl.Result{}, res) +} + func TestCreateOrUpdateResource_Run_SensitiveMissingKey(t *testing.T) { mctrl := gomock.NewController(t) msc := database.NewMockClient(mctrl) @@ -1102,3 +1261,286 @@ func TestCreateOrUpdateResource_Run_SensitiveMultipleFields(t *testing.T) { require.NoError(t, err) require.Equal(t, ctrl.Result{}, res) } + +func Test_buildSecretReferences(t *testing.T) { + const resourceID = "/planes/radius/local/resourceGroups/test-rg/providers/Applications.Test/testResources/my-resource" + + t.Run("resolves a top-level reference to a sibling secret ID", func(t *testing.T) { + properties := map[string]any{"secretName": "db-secret"} + + references, err := buildSecretReferences(resourceID, properties, []string{"secretName"}) + require.NoError(t, err) + require.Equal(t, map[string]string{ + "secretName": "/planes/radius/local/resourceGroups/test-rg/providers/Radius.Security/secrets/db-secret", + }, references) + }) + + t.Run("resolves a nested reference", func(t *testing.T) { + properties := map[string]any{"config": map[string]any{"secretName": "db-secret"}} + + references, err := buildSecretReferences(resourceID, properties, []string{"config.secretName"}) + require.NoError(t, err) + require.Equal(t, map[string]string{ + "config.secretName": "/planes/radius/local/resourceGroups/test-rg/providers/Radius.Security/secrets/db-secret", + }, references) + }) + + t.Run("skips an unset reference", func(t *testing.T) { + references, err := buildSecretReferences(resourceID, map[string]any{}, []string{"secretName"}) + require.NoError(t, err) + require.Nil(t, references) + }) + + t.Run("returns nil when there are no reference paths", func(t *testing.T) { + references, err := buildSecretReferences(resourceID, map[string]any{"secretName": "db-secret"}, nil) + require.NoError(t, err) + require.Nil(t, references) + }) + + t.Run("fails closed on an invalid resource ID", func(t *testing.T) { + _, err := buildSecretReferences("not-a-valid-id", map[string]any{"secretName": "db-secret"}, []string{"secretName"}) + require.Error(t, err) + }) +} + +func Test_buildSecretBindings(t *testing.T) { + const secretID = "/planes/radius/local/resourceGroups/test-rg/providers/Radius.Security/secrets/db-secret" + const tlsSecretID = "/planes/radius/local/resourceGroups/test-rg/providers/Radius.Security/secrets/tls-secret" + + t.Run("resolves an array of secret IDs", func(t *testing.T) { + properties := map[string]any{ + "secrets": []any{secretID, tlsSecretID}, + } + + bindings, err := buildSecretBindings(properties, []string{"secrets"}) + require.NoError(t, err) + require.Equal(t, []string{secretID, tlsSecretID}, bindings) + }) + + t.Run("resolves a nested array", func(t *testing.T) { + properties := map[string]any{ + "config": map[string]any{ + "secrets": []any{secretID}, + }, + } + + bindings, err := buildSecretBindings(properties, []string{"config.secrets"}) + require.NoError(t, err) + require.Equal(t, []string{secretID}, bindings) + }) + + t.Run("skips an unset property", func(t *testing.T) { + bindings, err := buildSecretBindings(map[string]any{}, []string{"secrets"}) + require.NoError(t, err) + require.Nil(t, bindings) + }) + + t.Run("returns nil when there are no binding paths", func(t *testing.T) { + bindings, err := buildSecretBindings(map[string]any{"secrets": []any{secretID}}, nil) + require.NoError(t, err) + require.Nil(t, bindings) + }) + + t.Run("fails closed when the property is not an array", func(t *testing.T) { + properties := map[string]any{ + "secrets": map[string]any{"username": secretID}, + } + + _, err := buildSecretBindings(properties, []string{"secrets"}) + require.Error(t, err) + require.Contains(t, err.Error(), "must be an array of secret IDs") + }) + + t.Run("fails closed on a non-string entry", func(t *testing.T) { + properties := map[string]any{ + "secrets": []any{secretID, 42}, + } + + _, err := buildSecretBindings(properties, []string{"secrets"}) + require.Error(t, err) + require.Contains(t, err.Error(), "must be a non-empty secret ID string") + }) + + t.Run("fails closed on an empty entry", func(t *testing.T) { + properties := map[string]any{ + "secrets": []any{""}, + } + + _, err := buildSecretBindings(properties, []string{"secrets"}) + require.Error(t, err) + require.Contains(t, err.Error(), "must be a non-empty secret ID string") + }) + + t.Run("fails closed on an invalid secret ID", func(t *testing.T) { + properties := map[string]any{ + "secrets": []any{"not-a-valid-id"}, + } + + _, err := buildSecretBindings(properties, []string{"secrets"}) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid secret ID") + }) +} + +func Test_stringValueAtPath(t *testing.T) { + properties := map[string]any{ + "secretName": "db-secret", + "config": map[string]any{ + "secretName": "nested-secret", + }, + "port": 5432, + } + + t.Run("top-level string", func(t *testing.T) { + value, ok := stringValueAtPath(properties, "secretName") + require.True(t, ok) + require.Equal(t, "db-secret", value) + }) + + t.Run("nested string", func(t *testing.T) { + value, ok := stringValueAtPath(properties, "config.secretName") + require.True(t, ok) + require.Equal(t, "nested-secret", value) + }) + + t.Run("missing path", func(t *testing.T) { + _, ok := stringValueAtPath(properties, "missing") + require.False(t, ok) + }) + + t.Run("non-string value", func(t *testing.T) { + _, ok := stringValueAtPath(properties, "port") + require.False(t, ok) + }) + + t.Run("path through a non-map", func(t *testing.T) { + _, ok := stringValueAtPath(properties, "secretName.deeper") + require.False(t, ok) + }) +} + +func Test_applySecretWriteBacks(t *testing.T) { + const boundSecretID = "/planes/radius/local/resourceGroups/radius-test-rg/providers/Radius.Security/secrets/kafkasecret" + const namespace = "test-namespace" + const newConnString = "Endpoint=sb://ns.servicebus.windows.net/;SharedAccessKeyName=Root;SharedAccessKey=abc123=" + + // Schema for Radius.Security/secrets: data is a map whose values carry a sensitive, retained `value`. + secretSchema := map[string]any{ + "properties": map[string]any{ + "data": map[string]any{ + "type": "object", + "additionalProperties": map[string]any{ + "type": "object", + "properties": map[string]any{ + "value": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + "x-radius-retain": true, + }, + }, + }, + }, + }, + } + + setup := func(t *testing.T) (*CreateOrUpdateResource[*TestResource, TestResource], *database.MockClient, runtimeclient.Client) { + t.Helper() + mctrl := gomock.NewController(t) + msc := database.NewMockClient(mctrl) + + key, err := encryption.GenerateKey() + require.NoError(t, err) + + ucpClient, err := testUCPClientFactory(secretSchema) + require.NoError(t, err) + + keySecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: encryption.DefaultEncryptionKeySecretName, + Namespace: encryption.RadiusNamespace, + }, + Data: map[string][]byte{ + encryption.DefaultEncryptionKeySecretKey: mustKeyStoreJSON(t, key), + }, + } + // The backing Kubernetes Secret the secret's own recipe would have materialized (empty placeholder). + backingSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kafkasecret", + Namespace: namespace, + Labels: map[string]string{"resource": "kafkasecret"}, + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{"connectionString": []byte("")}, + } + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + k8sClient := controllerfake.NewClientBuilder().WithScheme(scheme).WithObjects(keySecret, backingSecret).Build() + + opts := ctrl.Options{DatabaseClient: msc, UcpClient: ucpClient, KubeClient: k8sClient} + c, err := NewCreateOrUpdateResource(opts, successProcessorReference, engine.NewMockEngine(mctrl), configloader.NewMockConfigurationLoader(mctrl)) + require.NoError(t, err) + + return c.(*CreateOrUpdateResource[*TestResource, TestResource]), msc, k8sClient + } + + storedSecret := func() map[string]any { + return map[string]any{ + "id": boundSecretID, + "name": "kafkasecret", + "type": "Radius.Security/secrets", + "updatedApiVersion": "2025-08-01-preview", + "properties": map[string]any{ + "data": map[string]any{ + "connectionString": map[string]any{"value": "placeholder-encrypted", "encoding": "string"}, + }, + }, + } + } + + runtimeConfig := recipes.RuntimeConfiguration{Kubernetes: &recipes.KubernetesRuntime{Namespace: namespace}} + + t.Run("encrypts the value at rest and refreshes the backing Kubernetes Secret", func(t *testing.T) { + c, msc, k8sClient := setup(t) + + msc.EXPECT().Get(gomock.Any(), boundSecretID).Return( + &database.Object{Metadata: database.Metadata{ID: boundSecretID, ETag: "etag-secret"}, Data: storedSecret()}, nil) + + var savedValue map[string]any + msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, obj *database.Object, opts ...database.SaveOptions) error { + data := obj.Data.(map[string]any) + props := data["properties"].(map[string]any) + dataMap := props["data"].(map[string]any) + entry := dataMap["connectionString"].(map[string]any) + require.Equal(t, "string", entry["encoding"]) + savedValue = entry["value"].(map[string]any) + return nil + }) + + err := c.applySecretWriteBacks(context.Background(), []recipes.SecretWriteBack{ + {SecretID: boundSecretID, Data: map[string]string{"connectionString": newConnString}}, + }, runtimeConfig) + require.NoError(t, err) + + // The value persisted at rest is an encrypted envelope, not the plaintext. + require.Contains(t, savedValue, "encrypted") + require.Contains(t, savedValue, "nonce") + require.NotContains(t, fmt.Sprintf("%v", savedValue["encrypted"]), newConnString) + + // The backing Kubernetes Secret now carries the plaintext so a container can consume it. + updated := &corev1.Secret{} + require.NoError(t, k8sClient.Get(context.Background(), runtimeclient.ObjectKey{Namespace: namespace, Name: "kafkasecret"}, updated)) + require.Equal(t, newConnString, string(updated.Data["connectionString"])) + }) + + t.Run("fails closed when the namespace is missing", func(t *testing.T) { + c, _, _ := setup(t) + err := c.applySecretWriteBacks(context.Background(), []recipes.SecretWriteBack{ + {SecretID: boundSecretID, Data: map[string]string{"connectionString": newConnString}}, + }, recipes.RuntimeConfiguration{}) + require.Error(t, err) + require.Contains(t, err.Error(), "namespace") + }) +} diff --git a/pkg/recipes/configloader/environment.go b/pkg/recipes/configloader/environment.go index 3faab988ef1..3ecc34a3a24 100644 --- a/pkg/recipes/configloader/environment.go +++ b/pkg/recipes/configloader/environment.go @@ -385,6 +385,7 @@ func getRecipeDefinitionFromEnvironmentV20250801(ctx context.Context, environmen TemplateVersion: templateVersion, PlainHTTP: recipeDefinition.PlainHTTP, Outputs: recipeDefinition.Outputs, + SecretOutputs: recipeDefinition.SecretOutputs, } return definition, nil } @@ -472,11 +473,12 @@ func fetchRecipeDefinition(ctx context.Context, recipePackIDs []string, armOptio plainHTTP = *definition.PlainHTTP } return &recipes.RecipeDefinition{ - Kind: string(*definition.Kind), - Source: string(*definition.Source), - Parameters: definition.Parameters, - PlainHTTP: plainHTTP, - Outputs: to.StringMap(definition.Outputs), + Kind: string(*definition.Kind), + Source: string(*definition.Source), + Parameters: definition.Parameters, + PlainHTTP: plainHTTP, + Outputs: to.StringMap(definition.Outputs), + SecretOutputs: toRecipeSecretOutputs(definition.SecretOutputs), }, nil } } @@ -485,6 +487,23 @@ func fetchRecipeDefinition(ctx context.Context, recipePackIDs []string, armOptio return nil, fmt.Errorf("no recipe pack found with recipe for resource type %q", resourceType) } +// toRecipeSecretOutputs converts a versioned secretOutputs map (property name -> secret data key -> +// module output name, with pointer values) into the recipe definition representation. +func toRecipeSecretOutputs(in map[string]map[string]*string) map[string]map[string]string { + if in == nil { + return nil + } + out := make(map[string]map[string]string, len(in)) + for property, keys := range in { + inner := make(map[string]string, len(keys)) + for secretKey, outputName := range keys { + inner[secretKey] = to.String(outputName) + } + out[property] = inner + } + return out +} + // reconcileRecipeParameters merges recipe pack parameters with environment-level recipe parameters. // Environment-level parameters override recipe pack parameters when the same key exists. func reconcileRecipeParameters(recipePackParams map[string]any, envRecipeParams map[string]map[string]any, resourceType string) map[string]any { diff --git a/pkg/recipes/driver/bicep/bicep.go b/pkg/recipes/driver/bicep/bicep.go index 2d47e933a3e..83a5e77e251 100644 --- a/pkg/recipes/driver/bicep/bicep.go +++ b/pkg/recipes/driver/bicep/bicep.go @@ -125,6 +125,9 @@ func (d *bicepDriver) Execute(ctx context.Context, opts driver.ExecuteOptions) ( //update the recipe context with connected resources properties recipeContext.Resource.Connections = opts.Recipe.ConnectedResourcesProperties + // update the recipe context with secret material referenced through x-radius-secret-reference properties + recipeContext.Resource.Secrets = opts.Recipe.Secrets + // get the parameters after resolving the conflict between developer and operator parameters // if the recipe template also has the context parameter defined then add it to the parameter for deployment isContextParameterDefined := hasContextParameter(recipeData) @@ -137,8 +140,14 @@ func (d *bicepDriver) Execute(ctx context.Context, opts driver.ExecuteOptions) ( // Direct module — resolve {{context.*}} expressions, merge parameters, and wrap as ARM parameters. // Resource (developer) parameters take precedence over environment (operator) parameters. mergedParams := recipes_util.ShallowMergeParameters(opts.Definition.Parameters, opts.Recipe.Parameters) - resolvedParams := paramresolver.ResolveParameterExpressions(mergedParams, recipeContext) - parameters = wrapARMParameters(resolvedParams) + resolved, err := paramresolver.ResolveParameterExpressions(mergedParams, recipeContext) + if err != nil { + return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes_util.RecipeSetupError, recipes.GetErrorDetails(err)) + } + // Secret-sourced parameters (resolved.SecureKeys) are passed to the module's @secure() parameters. + // ARM scrubs @secure() parameter values from deployment history, and Radius never logs the + // parameter map, so no additional handling is required on the Bicep path. + parameters = wrapARMParameters(resolved.Values) } deploymentName := deploymentPrefix + strconv.FormatInt(time.Now().UnixNano(), 10) @@ -181,7 +190,7 @@ func (d *bicepDriver) Execute(ctx context.Context, opts driver.ExecuteOptions) ( return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, fmt.Sprintf("failed to deploy recipe %s of type %s", opts.BaseOptions.Recipe.Name, opts.BaseOptions.Definition.ResourceType), recipes_util.ExecutionError, recipes.GetErrorDetails(err)) } - recipeResponse, err := d.prepareRecipeResponse(opts.BaseOptions.Definition, resp.Properties.Outputs, resp.Properties.OutputResources) + recipeResponse, err := d.prepareRecipeResponse(opts.BaseOptions.Definition, opts.BaseOptions.Recipe.SecretBindings, resp.Properties.Outputs, resp.Properties.OutputResources) if err != nil { return nil, recipes.NewRecipeError(recipes.InvalidRecipeOutputs, fmt.Sprintf("failed to read the recipe output %q: %s", recipes.ResultPropertyName, err.Error()), recipes_util.ExecutionError, recipes.GetErrorDetails(err)) } @@ -400,7 +409,7 @@ func newProviderConfig(resourceGroup string, envProviders coredm.Providers) clie // // The latter is needed because non-ARM and non-UCP resources are not returned as part of the implicit 'resources' // collection. For us this mostly means Kubernetes resources - the user has to be explicit. -func (d *bicepDriver) prepareRecipeResponse(definition recipes.EnvironmentDefinition, outputs any, resources []*armdeployments.ResourceReference) (*recipes.RecipeOutput, error) { +func (d *bicepDriver) prepareRecipeResponse(definition recipes.EnvironmentDefinition, secretBindings []string, outputs any, resources []*armdeployments.ResourceReference) (*recipes.RecipeOutput, error) { recipeResponse := &recipes.RecipeOutput{} out, ok := outputs.(map[string]any) if ok && len(out) > 0 { @@ -428,6 +437,25 @@ func (d *bicepDriver) prepareRecipeResponse(definition recipes.EnvironmentDefini // secure-typed outputs to Secrets so they are not exposed as plain values. recipeResponse.Values, recipeResponse.Secrets = collectARMOutputs(out) } + + // Resolve any declared secretOutputs into write-back instructions using the RAW module outputs + // (before an outputs mapping is applied, which keeps only mapped outputs and would drop a secure + // output routed solely into a secret). The resource controller applies these, writing the secure + // module outputs back into the developer-authored Radius.Security/secrets resources bound to the + // resource. Consumed outputs are removed from the plain response so a value written into a secret + // is not also surfaced as a plain value or recipe secret. + if len(definition.SecretOutputs) > 0 { + rawValues, rawSecrets := collectARMOutputs(out) + writeBacks, consumed, err := recipes.ResolveSecretWriteBacks(rawValues, rawSecrets, definition.SecretOutputs, secretBindings) + if err != nil { + return &recipes.RecipeOutput{}, err + } + recipeResponse.SecretWriteBacks = writeBacks + for outputName := range consumed { + delete(recipeResponse.Values, outputName) + delete(recipeResponse.Secrets, outputName) + } + } } recipeResponse.Status = &rpv1.RecipeStatus{ diff --git a/pkg/recipes/driver/bicep/bicep_test.go b/pkg/recipes/driver/bicep/bicep_test.go index d142004c71c..9f3a19b45ca 100644 --- a/pkg/recipes/driver/bicep/bicep_test.go +++ b/pkg/recipes/driver/bicep/bicep_test.go @@ -314,7 +314,7 @@ func Test_Bicep_PrepareRecipeResponse_Success(t *testing.T) { }, PrevState: []string{}, } - actualResponse, err := d.prepareRecipeResponse(opts.BaseOptions.Definition, response, resources) + actualResponse, err := d.prepareRecipeResponse(opts.BaseOptions.Definition, nil, response, resources) require.NoError(t, err) require.Equal(t, expectedResponse, actualResponse) } @@ -351,7 +351,7 @@ func Test_Bicep_PrepareRecipeResponse_EmptySecret(t *testing.T) { }, } - actualResponse, err := d.prepareRecipeResponse(recipes.EnvironmentDefinition{TemplatePath: "radiusdev.azurecr.io/recipes/functionaltest/parameters/mongodatabases/azure:1.0"}, response, resources) + actualResponse, err := d.prepareRecipeResponse(recipes.EnvironmentDefinition{TemplatePath: "radiusdev.azurecr.io/recipes/functionaltest/parameters/mongodatabases/azure:1.0"}, nil, response, resources) require.NoError(t, err) require.Equal(t, expectedResponse, actualResponse) } @@ -373,7 +373,7 @@ func Test_Bicep_PrepareRecipeResponse_EmptyResult(t *testing.T) { }, } - actualResponse, err := d.prepareRecipeResponse(recipes.EnvironmentDefinition{TemplatePath: "radiusdev.azurecr.io/recipes/functionaltest/parameters/mongodatabases/azure:1.0"}, response, resources) + actualResponse, err := d.prepareRecipeResponse(recipes.EnvironmentDefinition{TemplatePath: "radiusdev.azurecr.io/recipes/functionaltest/parameters/mongodatabases/azure:1.0"}, nil, response, resources) require.NoError(t, err) require.Equal(t, expectedResponse, actualResponse) } @@ -739,7 +739,7 @@ func Test_Bicep_PrepareRecipeResponse_DirectModule(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - resp, err := d.prepareRecipeResponse(tt.definition, tt.outputs, tt.resources) + resp, err := d.prepareRecipeResponse(tt.definition, nil, tt.outputs, tt.resources) require.Equal(t, tt.expectedErr, err) require.Equal(t, tt.expectedResponse, resp) }) diff --git a/pkg/recipes/driver/terraform/terraform.go b/pkg/recipes/driver/terraform/terraform.go index e7461a233fd..197c7924a50 100644 --- a/pkg/recipes/driver/terraform/terraform.go +++ b/pkg/recipes/driver/terraform/terraform.go @@ -121,7 +121,7 @@ func (d *terraformDriver) Execute(ctx context.Context, opts driver.ExecuteOption return nil, recipes.NewRecipeError(recipes.RecipeDeploymentFailed, err.Error(), recipes_util.ExecutionError, recipes.GetErrorDetails(err)) } - recipeOutputs, err := d.prepareRecipeResponse(ctx, opts.BaseOptions.Definition, opts.Configuration, tfState) + recipeOutputs, err := d.prepareRecipeResponse(ctx, opts.BaseOptions.Definition, opts.BaseOptions.Recipe.SecretBindings, opts.Configuration, tfState) if err != nil { return nil, recipes.NewRecipeError(recipes.InvalidRecipeOutputs, fmt.Sprintf("failed to read the recipe output %q: %s", recipes.ResultPropertyName, err.Error()), recipes_util.ExecutionError, recipes.GetErrorDetails(err)) } @@ -187,7 +187,7 @@ func (d *terraformDriver) Delete(ctx context.Context, opts driver.DeleteOptions) // - If no `outputs` mapping exists and the module has a `result` output, the module is treated as a // wrapped recipe (existing behavior). // - If neither is present, all module outputs pass through unchanged. -func (d *terraformDriver) prepareRecipeResponse(ctx context.Context, definition recipes.EnvironmentDefinition, configuration recipes.Configuration, tfState *tfjson.State) (*recipes.RecipeOutput, error) { +func (d *terraformDriver) prepareRecipeResponse(ctx context.Context, definition recipes.EnvironmentDefinition, secretBindings []string, configuration recipes.Configuration, tfState *tfjson.State) (*recipes.RecipeOutput, error) { // We need to use reflect.DeepEqual to compare the struct that has a slice with an empty struct. // The reason is that Go does not allow comparison of structs that contain slices. // Please see: https://go.dev/ref/spec#Comparison_operators. @@ -219,6 +219,25 @@ func (d *terraformDriver) prepareRecipeResponse(ctx context.Context, definition recipeResponse.Values = values recipeResponse.Secrets = secrets } + + // Resolve any declared secretOutputs into write-back instructions using the raw module outputs + // (before an outputs mapping is applied, which keeps only mapped outputs and would drop a secure + // output routed solely into a secret). The resource controller applies these, writing the secure + // module outputs back into the developer-authored Radius.Security/secrets resources bound to the + // resource. Consumed outputs are removed from the plain response so a value written into a secret + // is not also surfaced as a plain value or recipe secret. + if len(definition.SecretOutputs) > 0 { + rawValues, rawSecrets := collectFlatOutputs(moduleOutputs) + writeBacks, consumed, err := recipes.ResolveSecretWriteBacks(rawValues, rawSecrets, definition.SecretOutputs, secretBindings) + if err != nil { + return &recipes.RecipeOutput{}, err + } + recipeResponse.SecretWriteBacks = writeBacks + for outputName := range consumed { + delete(recipeResponse.Values, outputName) + delete(recipeResponse.Secrets, outputName) + } + } } recipeResponse.Status = &rpv1.RecipeStatus{ diff --git a/pkg/recipes/driver/terraform/terraform_test.go b/pkg/recipes/driver/terraform/terraform_test.go index e4e4d73c0bb..1490861b502 100644 --- a/pkg/recipes/driver/terraform/terraform_test.go +++ b/pkg/recipes/driver/terraform/terraform_test.go @@ -889,7 +889,7 @@ func Test_Terraform_PrepareRecipeResponse(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - recipeResponse, err := d.prepareRecipeResponse(context.Background(), opts.BaseOptions.Definition, opts.Configuration, tt.state) + recipeResponse, err := d.prepareRecipeResponse(context.Background(), opts.BaseOptions.Definition, nil, opts.Configuration, tt.state) require.Equal(t, tt.expectedErr, err) require.Equal(t, tt.expectedResponse, recipeResponse) }) @@ -1038,7 +1038,7 @@ func Test_Terraform_PrepareRecipeResponse_DirectModule(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - recipeResponse, err := d.prepareRecipeResponse(context.Background(), tt.definition, recipes.Configuration{}, tt.state) + recipeResponse, err := d.prepareRecipeResponse(context.Background(), tt.definition, nil, recipes.Configuration{}, tt.state) require.Equal(t, tt.expectedErr, err) require.Equal(t, tt.expectedResponse, recipeResponse) }) diff --git a/pkg/recipes/engine/engine.go b/pkg/recipes/engine/engine.go index e981ed54e82..8a0a331e1fe 100644 --- a/pkg/recipes/engine/engine.go +++ b/pkg/recipes/engine/engine.go @@ -19,6 +19,7 @@ package engine import ( "context" "fmt" + "strings" "time" "github.com/radius-project/radius/pkg/components/metrics" @@ -27,6 +28,7 @@ import ( recipedriver "github.com/radius-project/radius/pkg/recipes/driver" "github.com/radius-project/radius/pkg/recipes/util" rpv1 "github.com/radius-project/radius/pkg/rp/v1" + "github.com/radius-project/radius/pkg/ucp/resources" "github.com/radius-project/radius/pkg/ucp/ucplog" ) @@ -96,6 +98,26 @@ func (e *engine) executeCore(ctx context.Context, recipe recipes.ResourceMetadat return nil, nil, err } + // Enrich secret-typed connections with their secret material so recipe parameter expressions + // (context.resource.connections..secrets.) can resolve developer-authored secrets. + e.enrichConnectionSecrets(ctx, &recipe) + + // Enrich x-radius-secret-reference properties with their referenced secret material so recipe + // parameter expressions (context.resource.secrets.) can resolve developer-authored secrets. + // Unlike connection enrichment, this is fail-closed: a referenced secret that cannot be loaded fails + // the deployment rather than passing an unresolved expression (a literal placeholder) to the module. + if err := e.enrichSecretReferences(ctx, &recipe); err != nil { + return nil, definition, recipes.NewRecipeError(recipes.RecipeConfigurationFailure, err.Error(), util.RecipeSetupError, recipes.GetErrorDetails(err)) + } + + // Enrich x-radius-secret-binding properties with their bound secret material so recipe parameter + // expressions (context.resource.secrets..) can resolve developer-authored secrets. Like + // enrichSecretReferences, this is fail-closed: a bound secret that cannot be loaded fails the deployment + // rather than passing an unresolved expression (a literal placeholder) to the module. + if err := e.enrichSecretBindings(ctx, &recipe); err != nil { + return nil, definition, recipes.NewRecipeError(recipes.RecipeConfigurationFailure, err.Error(), util.RecipeSetupError, recipes.GetErrorDetails(err)) + } + res, err := driver.Execute(ctx, recipedriver.ExecuteOptions{ BaseOptions: recipedriver.BaseOptions{ Configuration: *configuration, @@ -247,3 +269,155 @@ func (e *engine) getRecipeConfigSecrets(ctx context.Context, driver recipedriver return secretData, nil } + +// secretConnectionTypes is the set of connected-resource types whose values are sourced from the secret +// store rather than from non-secret properties. +var secretConnectionTypes = map[string]bool{ + "applications.core/secretstores": true, + "radius.security/secrets": true, +} + +// isSecretConnectionType reports whether a connected resource's type is a secret-typed resource. +func isSecretConnectionType(resourceType string) bool { + return secretConnectionTypes[strings.ToLower(resourceType)] +} + +// enrichConnectionSecrets loads secret material for secret-typed connected resources through the secret +// store and stores it on each connection's tainted Secrets field, so the parameter resolver can inject +// developer-authored secrets into module parameters. Enrichment is best-effort: if the secrets loader is +// unavailable or a secret cannot be loaded, the connection is left without secret data and any recipe +// expression that references it is left unresolved (consistent with other unresolved expressions) rather +// than failing the deployment. +func (e *engine) enrichConnectionSecrets(ctx context.Context, recipe *recipes.ResourceMetadata) { + logger := ucplog.FromContextOrDiscard(ctx) + if e.options.SecretsLoader == nil || recipe == nil { + return + } + + for name, conn := range recipe.ConnectedResourcesProperties { + if conn.ID == "" || !isSecretConnectionType(conn.Type) { + continue + } + + // A nil keys filter loads all secret keys for the secret store. + loaded, err := e.options.SecretsLoader.LoadSecrets(ctx, map[string][]string{conn.ID: nil}) + if err != nil { + logger.Info(fmt.Sprintf("skipping secret enrichment for connection %q: %s", name, err.Error())) + continue + } + + if data, ok := loaded[conn.ID]; ok { + conn.Secrets = data.Data + recipe.ConnectedResourcesProperties[name] = conn + } + } +} + +// enrichSecretReferences loads secret material for x-radius-secret-reference properties and stores it on +// the recipe's tainted Secrets field, so the parameter resolver can inject developer-authored secrets into +// module parameters via the context.resource.secrets. expression path. Unlike enrichConnectionSecrets, +// this is fail-closed: if a referenced secret cannot be loaded, an error is returned so the deployment fails +// rather than passing an unresolved expression (and therefore a literal placeholder) to the module. +func (e *engine) enrichSecretReferences(ctx context.Context, recipe *recipes.ResourceMetadata) error { + if recipe == nil || len(recipe.SecretReferences) == 0 { + return nil + } + + if e.options.SecretsLoader == nil { + return fmt.Errorf("secrets loader is not configured; cannot resolve referenced secrets") + } + + // Collect the distinct secret IDs referenced by the resource's properties. + secretIDs := map[string]bool{} + for _, secretID := range recipe.SecretReferences { + if secretID != "" { + secretIDs[secretID] = true + } + } + if len(secretIDs) == 0 { + return nil + } + + if recipe.Secrets == nil { + recipe.Secrets = map[string]string{} + } + + for secretID := range secretIDs { + // A nil keys filter loads all secret keys for the referenced secret. + loaded, err := e.options.SecretsLoader.LoadSecrets(ctx, map[string][]string{secretID: nil}) + if err != nil { + return fmt.Errorf("failed to load referenced secret %q: %w", secretID, err) + } + + data, ok := loaded[secretID] + if !ok { + return fmt.Errorf("referenced secret %q returned no data", secretID) + } + + for key, val := range data.Data { + recipe.Secrets[key] = val + } + } + + return nil +} + +// enrichSecretBindings loads every key of each secret listed in an x-radius-secret-binding array property and +// stores it on the recipe's tainted Secrets field under a "." namespace, so the parameter +// resolver can inject developer-authored secrets into module parameters via the +// context.resource.secrets.. expression path. The secret name is parsed from the bound +// resource ID. Like enrichSecretReferences, this is fail-closed: if a bound secret cannot be loaded, an error +// is returned so the deployment fails rather than passing an unresolved expression to the module. +func (e *engine) enrichSecretBindings(ctx context.Context, recipe *recipes.ResourceMetadata) error { + if recipe == nil || len(recipe.SecretBindings) == 0 { + return nil + } + + if e.options.SecretsLoader == nil { + return fmt.Errorf("secrets loader is not configured; cannot resolve bound secrets") + } + + // Collect the distinct secret IDs bound by the resource's properties. + secretIDs := map[string]bool{} + for _, secretID := range recipe.SecretBindings { + if secretID != "" { + secretIDs[secretID] = true + } + } + if len(secretIDs) == 0 { + return nil + } + + if recipe.Secrets == nil { + recipe.Secrets = map[string]string{} + } + + for secretID := range secretIDs { + parsed, err := resources.ParseResource(secretID) + if err != nil { + return fmt.Errorf("invalid bound secret ID %q: %w", secretID, err) + } + name := parsed.Name() + + // A nil keys filter loads all secret keys for the bound secret. + loaded, err := e.options.SecretsLoader.LoadSecrets(ctx, map[string][]string{secretID: nil}) + if err != nil { + return fmt.Errorf("failed to load bound secret %q: %w", secretID, err) + } + + data, ok := loaded[secretID] + if !ok { + return fmt.Errorf("bound secret %q returned no data", secretID) + } + + for key, val := range data.Data { + namespacedKey := name + "." + key + if existing, exists := recipe.Secrets[namespacedKey]; exists && existing != val { + return fmt.Errorf("bound secret namespace collision for %q: two secrets named %q provide different values for key %q", namespacedKey, name, key) + } + recipe.Secrets[namespacedKey] = val + } + } + + return nil +} diff --git a/pkg/recipes/engine/engine_test.go b/pkg/recipes/engine/engine_test.go index f443b9bb751..6d96ae075a9 100644 --- a/pkg/recipes/engine/engine_test.go +++ b/pkg/recipes/engine/engine_test.go @@ -1214,3 +1214,302 @@ func Test_Engine_Delete_With_Secrets_Success(t *testing.T) { }) require.NoError(t, err) } + +func Test_enrichConnectionSecrets(t *testing.T) { + const ( + secretStoreID = "/planes/radius/local/resourceGroups/test-rg/providers/Applications.Core/secretStores/db-credentials" + dbID = "/planes/radius/local/resourceGroups/test-rg/providers/Applications.Datastores/sqlDatabases/db" + ) + + newRecipe := func() *recipes.ResourceMetadata { + return &recipes.ResourceMetadata{ + ConnectedResourcesProperties: map[string]recipes.ConnectedResource{ + "credentials": { + ID: secretStoreID, + Name: "db-credentials", + Type: "Applications.Core/secretStores", + }, + "database": { + ID: dbID, + Name: "db", + Type: "Applications.Datastores/sqlDatabases", + }, + }, + } + } + + t.Run("populates secrets for secret-typed connections only", func(t *testing.T) { + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := newRecipe() + + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{secretStoreID: nil}). + Times(1). + Return(map[string]recipes.SecretData{ + secretStoreID: {Type: "generic", Data: map[string]string{"username": "admin", "password": "s3cr3t"}}, + }, nil) + + e.enrichConnectionSecrets(ctx, recipe) + + require.Equal(t, map[string]string{"username": "admin", "password": "s3cr3t"}, recipe.ConnectedResourcesProperties["credentials"].Secrets) + require.Nil(t, recipe.ConnectedResourcesProperties["database"].Secrets) + }) + + t.Run("load error is non-fatal and leaves connection without secrets", func(t *testing.T) { + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := newRecipe() + + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{secretStoreID: nil}). + Times(1). + Return(nil, errors.New("secret store not found")) + + e.enrichConnectionSecrets(ctx, recipe) + + require.Nil(t, recipe.ConnectedResourcesProperties["credentials"].Secrets) + }) + + t.Run("nil secrets loader is a no-op", func(t *testing.T) { + ctx := testcontext.New(t) + e := engine{options: Options{}} + recipe := newRecipe() + + require.NotPanics(t, func() { e.enrichConnectionSecrets(ctx, recipe) }) + require.Nil(t, recipe.ConnectedResourcesProperties["credentials"].Secrets) + }) +} + +func Test_enrichSecretReferences(t *testing.T) { + const secretID = "/planes/radius/local/resourceGroups/test-rg/providers/Radius.Security/secrets/db-secret" + + t.Run("populates secrets from referenced secret", func(t *testing.T) { + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := &recipes.ResourceMetadata{ + SecretReferences: map[string]string{"secretName": secretID}, + } + + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{secretID: nil}). + Times(1). + Return(map[string]recipes.SecretData{ + secretID: {Type: "Radius.Security/secrets", Data: map[string]string{"password": "s3cr3t"}}, + }, nil) + + err := e.enrichSecretReferences(ctx, recipe) + require.NoError(t, err) + require.Equal(t, map[string]string{"password": "s3cr3t"}, recipe.Secrets) + }) + + t.Run("load error fails closed", func(t *testing.T) { + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := &recipes.ResourceMetadata{ + SecretReferences: map[string]string{"secretName": secretID}, + } + + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{secretID: nil}). + Times(1). + Return(nil, errors.New("kubernetes secret not found")) + + err := e.enrichSecretReferences(ctx, recipe) + require.Error(t, err) + require.Contains(t, err.Error(), "kubernetes secret not found") + require.Empty(t, recipe.Secrets) + }) + + t.Run("missing data for referenced secret fails closed", func(t *testing.T) { + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := &recipes.ResourceMetadata{ + SecretReferences: map[string]string{"secretName": secretID}, + } + + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{secretID: nil}). + Times(1). + Return(map[string]recipes.SecretData{}, nil) + + err := e.enrichSecretReferences(ctx, recipe) + require.Error(t, err) + require.Contains(t, err.Error(), "returned no data") + }) + + t.Run("nil secrets loader with references fails closed", func(t *testing.T) { + ctx := testcontext.New(t) + e := engine{options: Options{}} + recipe := &recipes.ResourceMetadata{ + SecretReferences: map[string]string{"secretName": secretID}, + } + + err := e.enrichSecretReferences(ctx, recipe) + require.Error(t, err) + require.Contains(t, err.Error(), "secrets loader is not configured") + }) + + t.Run("no secret references is a no-op", func(t *testing.T) { + ctx := testcontext.New(t) + e := engine{options: Options{}} + recipe := &recipes.ResourceMetadata{} + + err := e.enrichSecretReferences(ctx, recipe) + require.NoError(t, err) + require.Nil(t, recipe.Secrets) + }) +} + +func Test_enrichSecretBindings(t *testing.T) { + const secretID = "/planes/radius/local/resourceGroups/test-rg/providers/Radius.Security/secrets/db-secret" + const tlsSecretID = "/planes/radius/local/resourceGroups/test-rg/providers/Radius.Security/secrets/tls-secret" + + t.Run("namespaces all keys of a bound secret by its name", func(t *testing.T) { + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := &recipes.ResourceMetadata{ + SecretBindings: []string{secretID}, + } + + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{secretID: nil}). + Times(1). + Return(map[string]recipes.SecretData{ + secretID: {Type: "Radius.Security/secrets", Data: map[string]string{"USERNAME": "radadmin", "PASSWORD": "s3cr3t"}}, + }, nil) + + err := e.enrichSecretBindings(ctx, recipe) + require.NoError(t, err) + require.Equal(t, map[string]string{"db-secret.USERNAME": "radadmin", "db-secret.PASSWORD": "s3cr3t"}, recipe.Secrets) + }) + + t.Run("namespaces multiple secrets distinctly", func(t *testing.T) { + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := &recipes.ResourceMetadata{ + SecretBindings: []string{secretID, tlsSecretID}, + } + + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{secretID: nil}). + Times(1). + Return(map[string]recipes.SecretData{ + secretID: {Type: "Radius.Security/secrets", Data: map[string]string{"USERNAME": "radadmin"}}, + }, nil) + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{tlsSecretID: nil}). + Times(1). + Return(map[string]recipes.SecretData{ + tlsSecretID: {Type: "Radius.Security/secrets", Data: map[string]string{"USERNAME": "tlsuser", "CA_CERT": "pem"}}, + }, nil) + + err := e.enrichSecretBindings(ctx, recipe) + require.NoError(t, err) + require.Equal(t, map[string]string{ + "db-secret.USERNAME": "radadmin", + "tls-secret.USERNAME": "tlsuser", + "tls-secret.CA_CERT": "pem", + }, recipe.Secrets) + }) + + t.Run("load error fails closed", func(t *testing.T) { + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := &recipes.ResourceMetadata{ + SecretBindings: []string{secretID}, + } + + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{secretID: nil}). + Times(1). + Return(nil, errors.New("kubernetes secret not found")) + + err := e.enrichSecretBindings(ctx, recipe) + require.Error(t, err) + require.Contains(t, err.Error(), "kubernetes secret not found") + require.Empty(t, recipe.Secrets) + }) + + t.Run("invalid secret ID fails closed", func(t *testing.T) { + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := &recipes.ResourceMetadata{ + SecretBindings: []string{"not-a-valid-id"}, + } + + err := e.enrichSecretBindings(ctx, recipe) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid bound secret ID") + }) + + t.Run("namespace collision fails closed", func(t *testing.T) { + // Two distinct secret IDs whose resource names are identical and whose data disagree on a key. + const dupA = "/planes/radius/local/resourceGroups/rg-a/providers/Radius.Security/secrets/db-secret" + const dupB = "/planes/radius/local/resourceGroups/rg-b/providers/Radius.Security/secrets/db-secret" + ctrl := gomock.NewController(t) + secretsLoader := configloader.NewMockSecretsLoader(ctrl) + ctx := testcontext.New(t) + e := engine{options: Options{SecretsLoader: secretsLoader}} + recipe := &recipes.ResourceMetadata{ + SecretBindings: []string{dupA, dupB}, + } + + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{dupA: nil}). + Times(1). + Return(map[string]recipes.SecretData{ + dupA: {Type: "Radius.Security/secrets", Data: map[string]string{"USERNAME": "a"}}, + }, nil) + secretsLoader.EXPECT(). + LoadSecrets(ctx, map[string][]string{dupB: nil}). + Times(1). + Return(map[string]recipes.SecretData{ + dupB: {Type: "Radius.Security/secrets", Data: map[string]string{"USERNAME": "b"}}, + }, nil) + + err := e.enrichSecretBindings(ctx, recipe) + require.Error(t, err) + require.Contains(t, err.Error(), "namespace collision") + }) + + t.Run("nil secrets loader with bindings fails closed", func(t *testing.T) { + ctx := testcontext.New(t) + e := engine{options: Options{}} + recipe := &recipes.ResourceMetadata{ + SecretBindings: []string{secretID}, + } + + err := e.enrichSecretBindings(ctx, recipe) + require.Error(t, err) + require.Contains(t, err.Error(), "secrets loader is not configured") + }) + + t.Run("no bindings is a no-op", func(t *testing.T) { + ctx := testcontext.New(t) + e := engine{options: Options{}} + recipe := &recipes.ResourceMetadata{} + + err := e.enrichSecretBindings(ctx, recipe) + require.NoError(t, err) + require.Nil(t, recipe.Secrets) + }) +} diff --git a/pkg/recipes/paramresolver/resolver.go b/pkg/recipes/paramresolver/resolver.go index 0348cd34b13..0afda014465 100644 --- a/pkg/recipes/paramresolver/resolver.go +++ b/pkg/recipes/paramresolver/resolver.go @@ -35,50 +35,100 @@ var singleExpressionPattern = regexp.MustCompile(`^\s*\{\{([^}]+)\}\}\s*$`) // conditionPattern matches a single ternary condition of the form: == "value". var conditionPattern = regexp.MustCompile(`^\s*(.+?)\s*==\s*"([^"]*)"\s*$`) +// ResolvedParameters holds resolved recipe parameters together with metadata identifying which +// parameters resolved from secret material, so drivers can route those values through secure channels +// (ARM @secure parameters / Terraform sensitive variables). +type ResolvedParameters struct { + // Values holds the resolved parameter values. + Values map[string]any + // SecureKeys is the set of top-level parameter names whose value resolved from a secret expression. + SecureKeys map[string]bool +} + // ResolveParameterExpressions resolves {{context.*}} template expressions in recipe parameters. // It traverses the parameter map recursively and replaces expressions with values from the // recipe context. Unrecognized expressions are left unchanged so that misconfigurations surface // as IaC engine errors rather than being silently masked. -func ResolveParameterExpressions(params map[string]any, ctx *recipecontext.Context) map[string]any { +// +// Secret expressions (context.resource.connections..secrets.) are resolved from a separate +// secret lookup and may only be used as the entire value of a parameter; interpolating a secret into a +// surrounding string returns an error so partial cleartext cannot leak into a non-secure value. Each +// parameter whose value resolves from a secret is reported in SecureKeys. +func ResolveParameterExpressions(params map[string]any, ctx *recipecontext.Context) (ResolvedParameters, error) { if params == nil { - return nil + return ResolvedParameters{}, nil } lookup := buildContextLookup(ctx) typedLookup := buildTypedContextLookup(ctx) + secretLookup := buildSecretLookup(ctx) + result := make(map[string]any, len(params)) + secureKeys := map[string]bool{} for k, v := range params { - result[k] = resolveValue(v, lookup, typedLookup) + resolved, secure, err := resolveValue(v, lookup, typedLookup, secretLookup) + if err != nil { + return ResolvedParameters{}, fmt.Errorf("failed to resolve parameter %q: %w", k, err) + } + result[k] = resolved + if secure { + secureKeys[k] = true + } } - return result + return ResolvedParameters{Values: result, SecureKeys: secureKeys}, nil } -// resolveValue resolves template expressions in a single value. It handles strings, maps, and slices recursively. -func resolveValue(v any, lookup map[string]string, typedLookup map[string]any) any { +// resolveValue resolves template expressions in a single value. It handles strings, maps, and slices +// recursively. The returned secure flag reports whether the value (or any nested value) resolved from a +// secret expression, so the caller can tag the top-level parameter for secure routing. +func resolveValue(v any, lookup map[string]string, typedLookup map[string]any, secretLookup map[string]string) (any, bool, error) { switch val := v.(type) { case string: + // A secret may only be referenced as the entire value of a parameter. When the whole string is a + // single {{...secrets...}} expression, inject the secret value and tag the parameter secure. + if m := singleExpressionPattern.FindStringSubmatch(val); m != nil { + if secret, ok := secretLookup[strings.TrimSpace(m[1])]; ok { + return secret, true, nil + } + } // When the entire value is a single {{...}} expression that maps to a typed context value, // preserve the original scalar type so typed module parameters (int, bool, object) are passed // through correctly instead of being coerced to a string. Interpolated strings and ternary // expressions fall back to string resolution below. if typed, ok := resolveTypedExpression(val, typedLookup); ok { - return typed + return typed, false, nil + } + resolved, err := resolveString(val, lookup, secretLookup) + if err != nil { + return nil, false, err } - return resolveString(val, lookup) + return resolved, false, nil case map[string]any: resolved := make(map[string]any, len(val)) + secure := false for k, inner := range val { - resolved[k] = resolveValue(inner, lookup, typedLookup) + r, s, err := resolveValue(inner, lookup, typedLookup, secretLookup) + if err != nil { + return nil, false, err + } + resolved[k] = r + secure = secure || s } - return resolved + return resolved, secure, nil case []any: resolved := make([]any, len(val)) + secure := false for i, inner := range val { - resolved[i] = resolveValue(inner, lookup, typedLookup) + r, s, err := resolveValue(inner, lookup, typedLookup, secretLookup) + if err != nil { + return nil, false, err + } + resolved[i] = r + secure = secure || s } - return resolved + return resolved, secure, nil default: - return v + return v, false, nil } } @@ -97,11 +147,23 @@ func resolveTypedExpression(s string, typedLookup map[string]any) (any, bool) { return val, ok } -// resolveString replaces all {{...}} expressions in a string with their resolved values. -func resolveString(s string, lookup map[string]string) string { - return expressionPattern.ReplaceAllStringFunc(s, func(match string) string { +// resolveString replaces all {{...}} expressions in a string with their resolved values. It returns an +// error if a secret expression is used in interpolation (i.e. embedded in surrounding text), because the +// whole-value secret case is handled by the caller and any secret reaching this path would leak partial +// cleartext into a non-secure value. +func resolveString(s string, lookup map[string]string, secretLookup map[string]string) (string, error) { + var resolveErr error + out := expressionPattern.ReplaceAllStringFunc(s, func(match string) string { // Strip the surrounding {{ and }}. inner := match[2 : len(match)-2] + key := strings.TrimSpace(inner) + + // A secret reaching the interpolation path is embedded in surrounding text (the whole-value + // case is handled in resolveValue). Reject it so partial cleartext cannot leak. + if _, isSecret := secretLookup[key]; isSecret { + resolveErr = fmt.Errorf("secret expression %q may only be used as the entire parameter value, not interpolated into a string", key) + return match + } // Try ternary evaluation first. if result, ok := evaluateTernary(inner, lookup); ok { @@ -109,7 +171,6 @@ func resolveString(s string, lookup map[string]string) string { } // Simple context path lookup. - key := strings.TrimSpace(inner) if val, ok := lookup[key]; ok { return val } @@ -117,6 +178,10 @@ func resolveString(s string, lookup map[string]string) string { // Unrecognized expression — leave unchanged. return match }) + if resolveErr != nil { + return "", resolveErr + } + return out, nil } // evaluateTernary evaluates a ternary expression of the form: @@ -306,3 +371,27 @@ func buildTypedContextLookup(ctx *recipecontext.Context) map[string]any { return typed } + +// buildSecretLookup builds a flat key-value map of secret material exposed by secret-typed connected +// resources and by x-radius-secret-reference properties. Keys use the paths +// context.resource.connections..secrets. and context.resource.secrets.. This lookup is +// kept separate from the non-secret context lookup so secret values can only be resolved through the +// whole-value secret path and are tagged for secure routing, never resolved as ordinary string values. +func buildSecretLookup(ctx *recipecontext.Context) map[string]string { + secrets := map[string]string{} + if ctx == nil { + return secrets + } + + for connName, conn := range ctx.Resource.Connections { + for key, val := range conn.Secrets { + secrets[fmt.Sprintf("context.resource.connections.%s.secrets.%s", connName, key)] = val + } + } + + for key, val := range ctx.Resource.Secrets { + secrets[fmt.Sprintf("context.resource.secrets.%s", key)] = val + } + + return secrets +} diff --git a/pkg/recipes/paramresolver/resolver_test.go b/pkg/recipes/paramresolver/resolver_test.go index f85ee2cc1ac..a36d2e9263c 100644 --- a/pkg/recipes/paramresolver/resolver_test.go +++ b/pkg/recipes/paramresolver/resolver_test.go @@ -46,8 +46,15 @@ func testContext() *recipecontext.Context { Properties: map[string]any{ "connectionString": "postgres://myhost:5432/mydb", }, + Secrets: map[string]string{ + "username": "admin", + "password": "s3cr3t-p@ss", + }, }, }, + Secrets: map[string]string{ + "apiKey": "k3y-v@lue", + }, }, Application: recipecontext.ResourceInfo{ Name: "my-app", @@ -303,11 +310,12 @@ func Test_ResolveParameterExpressions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := ResolveParameterExpressions(tt.params, tt.ctx) + result, err := ResolveParameterExpressions(tt.params, tt.ctx) + require.NoError(t, err) if tt.expected == nil { - assert.Nil(t, result) + assert.Nil(t, result.Values) } else { - require.Equal(t, tt.expected, result) + require.Equal(t, tt.expected, result.Values) } }) } @@ -437,8 +445,128 @@ func Test_TernaryExpressions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := ResolveParameterExpressions(tt.params, tt.ctx) - require.Equal(t, tt.expected, result) + result, err := ResolveParameterExpressions(tt.params, tt.ctx) + require.NoError(t, err) + require.Equal(t, tt.expected, result.Values) + }) + } +} + +func Test_SecretExpressions(t *testing.T) { + tests := []struct { + name string + params map[string]any + expectedValues map[string]any + expectedSecureKeys map[string]bool + expectErr bool + errContains string + }{ + { + name: "whole-value secret resolves and is tagged secure", + params: map[string]any{ + "administratorLogin": "{{context.resource.connections.db.secrets.username}}", + "administratorLoginPassword": "{{context.resource.connections.db.secrets.password}}", + "sku": "Standard", + }, + expectedValues: map[string]any{ + "administratorLogin": "admin", + "administratorLoginPassword": "s3cr3t-p@ss", + "sku": "Standard", + }, + expectedSecureKeys: map[string]bool{ + "administratorLogin": true, + "administratorLoginPassword": true, + }, + }, + { + name: "whole-value secret with surrounding whitespace resolves", + params: map[string]any{ + "password": " {{ context.resource.connections.db.secrets.password }} ", + }, + expectedValues: map[string]any{ + "password": "s3cr3t-p@ss", + }, + expectedSecureKeys: map[string]bool{"password": true}, + }, + { + name: "secret nested in object value tags the top-level parameter secure", + params: map[string]any{ + "config": map[string]any{ + "user": "{{context.resource.connections.db.secrets.username}}", + "host": "{{context.resource.properties.host}}", + }, + }, + expectedValues: map[string]any{ + "config": map[string]any{ + "user": "admin", + "host": "myhost.example.com", + }, + }, + expectedSecureKeys: map[string]bool{"config": true}, + }, + { + name: "non-secret parameters are not tagged secure", + params: map[string]any{ + "host": "{{context.resource.properties.host}}", + "name": "{{context.resource.name}}", + }, + expectedValues: map[string]any{ + "host": "myhost.example.com", + "name": "my-resource", + }, + expectedSecureKeys: map[string]bool{}, + }, + { + name: "secret interpolated into a surrounding string is rejected", + params: map[string]any{ + "connectionString": "user=admin;password={{context.resource.connections.db.secrets.password}};", + }, + expectErr: true, + errContains: "may only be used as the entire parameter value", + }, + { + name: "reference to a missing secret key is left unchanged", + params: map[string]any{ + "token": "{{context.resource.connections.db.secrets.missing}}", + }, + expectedValues: map[string]any{ + "token": "{{context.resource.connections.db.secrets.missing}}", + }, + expectedSecureKeys: map[string]bool{}, + }, + { + name: "secret-reference secret resolves and is tagged secure", + params: map[string]any{ + "apiKey": "{{context.resource.secrets.apiKey}}", + "sku": "Standard", + }, + expectedValues: map[string]any{ + "apiKey": "k3y-v@lue", + "sku": "Standard", + }, + expectedSecureKeys: map[string]bool{"apiKey": true}, + }, + { + name: "secret-reference secret interpolated into a surrounding string is rejected", + params: map[string]any{ + "header": "Bearer {{context.resource.secrets.apiKey}}", + }, + expectErr: true, + errContains: "may only be used as the entire parameter value", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ResolveParameterExpressions(tt.params, testContext()) + if tt.expectErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errContains) + return + } + require.NoError(t, err) + require.Equal(t, tt.expectedValues, result.Values) + require.Equal(t, tt.expectedSecureKeys, result.SecureKeys) }) } } diff --git a/pkg/recipes/recipecontext/types.go b/pkg/recipes/recipecontext/types.go index 4736f5b95f3..52256c62262 100644 --- a/pkg/recipes/recipecontext/types.go +++ b/pkg/recipes/recipecontext/types.go @@ -62,6 +62,12 @@ type Resource struct { // context.resource.connections.[connection-name].name // context.resource.connections.[connection-name].type Connections map[string]recipes.ConnectedResource `json:"connections,omitempty"` + + // Secrets holds resolved secret material referenced by the resource through x-radius-secret-reference + // properties, keyed by secret key. It is tagged json:"-" so it is never serialized into the recipe + // context, logs, or IaC state. Recipe authors reference it via the context.resource.secrets. + // expression path, and resolved values are routed to @secure()/sensitive module parameters. + Secrets map[string]string `json:"-"` } // ResourceInfo represents name and id of the resource diff --git a/pkg/recipes/secretwriteback.go b/pkg/recipes/secretwriteback.go new file mode 100644 index 00000000000..39302b58a36 --- /dev/null +++ b/pkg/recipes/secretwriteback.go @@ -0,0 +1,106 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package recipes + +import ( + "fmt" + "sort" + + "github.com/radius-project/radius/pkg/ucp/resources" +) + +// ResolveSecretWriteBacks turns a recipe definition's SecretOutputs mapping into concrete write-back +// instructions (the "backwards" secret flow). For each entry — keyed by the name of a +// Radius.Security/secrets resource the deploying resource binds via an x-radius-secret-binding array +// property — it matches the bound secret ID (by name) from secretBindings and pairs each declared secret +// data key with the value of the named module output (looked up in secrets first, then values). It returns +// the resolved write-backs plus the set of module output names consumed, so the driver can drop them from +// the plain recipe response (a value written into a secret must not also be surfaced as an output). +// +// It operates on the module's RAW outputs (before any `outputs` mapping is applied), because that mapping +// keeps only explicitly mapped outputs — a secure output routed exclusively into a secret would otherwise +// be dropped before reaching the caller. It is fail-closed: a SecretOutputs entry naming a secret the +// resource does not bind, or a module output the recipe did not produce, is an error rather than a silently +// unpopulated secret. +func ResolveSecretWriteBacks(values map[string]any, secrets map[string]any, secretOutputs map[string]map[string]string, secretBindings []string) ([]SecretWriteBack, map[string]struct{}, error) { + if len(secretOutputs) == 0 { + return nil, nil, nil + } + + // Index the resource's bound secret IDs by resource name so a SecretOutputs entry (keyed by secret + // name) resolves to a fully qualified secret ID. + boundByName := map[string]string{} + for _, secretID := range secretBindings { + if secretID == "" { + continue + } + parsed, err := resources.ParseResource(secretID) + if err != nil { + return nil, nil, fmt.Errorf("invalid bound secret ID %q: %w", secretID, err) + } + boundByName[parsed.Name()] = secretID + } + + // Sort secret names for deterministic ordering. + secretNames := make([]string, 0, len(secretOutputs)) + for name := range secretOutputs { + secretNames = append(secretNames, name) + } + sort.Strings(secretNames) + + writeBacks := make([]SecretWriteBack, 0, len(secretNames)) + consumed := map[string]struct{}{} + + for _, secretName := range secretNames { + secretID, ok := boundByName[secretName] + if !ok { + return nil, nil, fmt.Errorf("recipe declares secretOutputs for secret %q but the resource does not bind a Radius.Security/secrets named %q (add it to the resource's secrets array)", secretName, secretName) + } + + keyMap := secretOutputs[secretName] + + // Sort data keys for deterministic ordering. + dataKeys := make([]string, 0, len(keyMap)) + for k := range keyMap { + dataKeys = append(dataKeys, k) + } + sort.Strings(dataKeys) + + data := make(map[string]string, len(keyMap)) + for _, dataKey := range dataKeys { + outputName := keyMap[dataKey] + val, ok := secrets[outputName] + if !ok { + val, ok = values[outputName] + } + if !ok { + return nil, nil, fmt.Errorf("recipe declares secretOutputs %s.%s but the module produced no output named %q", secretName, dataKey, outputName) + } + + strVal, ok := val.(string) + if !ok { + strVal = fmt.Sprintf("%v", val) + } + data[dataKey] = strVal + consumed[outputName] = struct{}{} + } + + writeBacks = append(writeBacks, SecretWriteBack{SecretID: secretID, Data: data}) + } + + return writeBacks, consumed, nil +} diff --git a/pkg/recipes/secretwriteback_test.go b/pkg/recipes/secretwriteback_test.go new file mode 100644 index 00000000000..53cd235043c --- /dev/null +++ b/pkg/recipes/secretwriteback_test.go @@ -0,0 +1,162 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package recipes + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +const ( + kafkaSecretID = "/planes/radius/local/resourceGroups/default/providers/Radius.Security/secrets/kafkasecret" + otherSecretID = "/planes/radius/local/resourceGroups/default/providers/Radius.Security/secrets/othersecret" +) + +func Test_ResolveSecretWriteBacks_NoSecretOutputs(t *testing.T) { + writeBacks, consumed, err := ResolveSecretWriteBacks( + map[string]any{"host": "example"}, + map[string]any{"primaryConnectionString": "secret-value"}, + nil, + []string{kafkaSecretID}, + ) + require.NoError(t, err) + require.Nil(t, writeBacks) + require.Nil(t, consumed) +} + +func Test_ResolveSecretWriteBacks_FromSecrets(t *testing.T) { + writeBacks, consumed, err := ResolveSecretWriteBacks( + map[string]any{"host": "example"}, + map[string]any{"primaryConnectionString": "secret-value"}, + map[string]map[string]string{ + "kafkasecret": {"connectionString": "primaryConnectionString"}, + }, + []string{kafkaSecretID}, + ) + require.NoError(t, err) + require.Equal(t, []SecretWriteBack{ + {SecretID: kafkaSecretID, Data: map[string]string{"connectionString": "secret-value"}}, + }, writeBacks) + require.Contains(t, consumed, "primaryConnectionString") +} + +func Test_ResolveSecretWriteBacks_FromValues(t *testing.T) { + // The referenced module output is a non-sensitive value (not a secret); it should still be resolved. + writeBacks, consumed, err := ResolveSecretWriteBacks( + map[string]any{"endpoint": "kafka:9092"}, + map[string]any{}, + map[string]map[string]string{ + "kafkasecret": {"broker": "endpoint"}, + }, + []string{kafkaSecretID}, + ) + require.NoError(t, err) + require.Equal(t, []SecretWriteBack{ + {SecretID: kafkaSecretID, Data: map[string]string{"broker": "kafka:9092"}}, + }, writeBacks) + require.Contains(t, consumed, "endpoint") +} + +func Test_ResolveSecretWriteBacks_SecretsPreferredOverValues(t *testing.T) { + writeBacks, _, err := ResolveSecretWriteBacks( + map[string]any{"conn": "from-values"}, + map[string]any{"conn": "from-secrets"}, + map[string]map[string]string{ + "kafkasecret": {"connectionString": "conn"}, + }, + []string{kafkaSecretID}, + ) + require.NoError(t, err) + require.Equal(t, "from-secrets", writeBacks[0].Data["connectionString"]) +} + +func Test_ResolveSecretWriteBacks_NonStringCoerced(t *testing.T) { + writeBacks, _, err := ResolveSecretWriteBacks( + map[string]any{"port": 9092}, + map[string]any{}, + map[string]map[string]string{ + "kafkasecret": {"port": "port"}, + }, + []string{kafkaSecretID}, + ) + require.NoError(t, err) + require.Equal(t, "9092", writeBacks[0].Data["port"]) +} + +func Test_ResolveSecretWriteBacks_MultipleSecretsAndKeys(t *testing.T) { + writeBacks, consumed, err := ResolveSecretWriteBacks( + map[string]any{}, + map[string]any{ + "connOut": "conn-value", + "userOut": "user-value", + "passOut": "pass-value", + }, + map[string]map[string]string{ + "othersecret": {"conn": "connOut"}, + "kafkasecret": {"username": "userOut", "password": "passOut"}, + }, + []string{kafkaSecretID, otherSecretID}, + ) + require.NoError(t, err) + // Deterministic ordering: secret names sorted (kafkasecret before othersecret). + require.Equal(t, []SecretWriteBack{ + {SecretID: kafkaSecretID, Data: map[string]string{"username": "user-value", "password": "pass-value"}}, + {SecretID: otherSecretID, Data: map[string]string{"conn": "conn-value"}}, + }, writeBacks) + require.Len(t, consumed, 3) +} + +func Test_ResolveSecretWriteBacks_UnboundSecret_FailsClosed(t *testing.T) { + _, _, err := ResolveSecretWriteBacks( + map[string]any{}, + map[string]any{"primaryConnectionString": "secret-value"}, + map[string]map[string]string{ + "kafkasecret": {"connectionString": "primaryConnectionString"}, + }, + // The resource does not bind kafkasecret. + []string{otherSecretID}, + ) + require.Error(t, err) + require.Contains(t, err.Error(), "does not bind") +} + +func Test_ResolveSecretWriteBacks_MissingOutput_FailsClosed(t *testing.T) { + _, _, err := ResolveSecretWriteBacks( + map[string]any{}, + map[string]any{}, + map[string]map[string]string{ + "kafkasecret": {"connectionString": "primaryConnectionString"}, + }, + []string{kafkaSecretID}, + ) + require.Error(t, err) + require.Contains(t, err.Error(), "produced no output named") +} + +func Test_ResolveSecretWriteBacks_InvalidBoundSecretID(t *testing.T) { + _, _, err := ResolveSecretWriteBacks( + map[string]any{}, + map[string]any{"primaryConnectionString": "secret-value"}, + map[string]map[string]string{ + "kafkasecret": {"connectionString": "primaryConnectionString"}, + }, + []string{"not-a-resource-id"}, + ) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid bound secret ID") +} diff --git a/pkg/recipes/terraform/config/config.go b/pkg/recipes/terraform/config/config.go index 630f487f815..3eea751df9d 100644 --- a/pkg/recipes/terraform/config/config.go +++ b/pkg/recipes/terraform/config/config.go @@ -199,6 +199,60 @@ func (cfg *TerraformConfig) AddRecipeContext(ctx context.Context, moduleName str return nil } +// SetModuleParams sets the resolved recipe parameters on the named module, routing any parameter named +// in secureKeys through a Terraform input variable marked sensitive = true. The module argument then +// references that variable (${var.}) so Terraform redacts the value from plan/apply output. +// Non-secure parameters are set inline. Nil values are skipped so a module variable's default is not +// overridden with an explicit null. +func (cfg *TerraformConfig) SetModuleParams(moduleName string, params RecipeParams, secureKeys map[string]bool) { + mod, ok := cfg.Module[moduleName] + if !ok { + return + } + + for k, v := range params { + if v == nil { + continue + } + + if secureKeys[k] { + varName := secureVariableName(moduleName, k) + if cfg.Variable == nil { + cfg.Variable = map[string]any{} + } + cfg.Variable[varName] = map[string]any{ + "type": "string", + "sensitive": true, + "default": v, + } + mod[k] = fmt.Sprintf("${var.%s}", varName) + continue + } + + mod[k] = v + } +} + +// secureVariableName builds a deterministic, valid Terraform identifier for the sensitive variable that +// backs a secret-sourced module parameter, namespaced by module and parameter name to avoid collisions. +func secureVariableName(moduleName, paramKey string) string { + return "radius_secret_" + sanitizeTerraformIdentifier(moduleName) + "_" + sanitizeTerraformIdentifier(paramKey) +} + +// sanitizeTerraformIdentifier replaces any character that is not a letter, digit, or underscore with an +// underscore so the result is a valid Terraform identifier component. +func sanitizeTerraformIdentifier(s string) string { + out := make([]rune, 0, len(s)) + for _, r := range s { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' { + out = append(out, r) + } else { + out = append(out, '_') + } + } + return string(out) +} + // newModuleConfig creates a new TFModuleConfig object with the given module source and version // and also populates RecipeParams in TF module config. If same parameter key exists across params // then the last map specified gets precedence. diff --git a/pkg/recipes/terraform/config/config_test.go b/pkg/recipes/terraform/config/config_test.go index ce5db89b223..a0c389bfa4d 100644 --- a/pkg/recipes/terraform/config/config_test.go +++ b/pkg/recipes/terraform/config/config_test.go @@ -783,6 +783,55 @@ func Test_AddMappedOutputs(t *testing.T) { } } +func Test_SetModuleParams(t *testing.T) { + envRecipe, resourceRecipe := getTestInputs() + + t.Run("routes secure params through sensitive variables and sets others inline", func(t *testing.T) { + tfconfig, err := New(context.Background(), testRecipeName, &envRecipe, &resourceRecipe) + require.NoError(t, err) + + params := RecipeParams{ + "administratorLogin": "admin", + "administratorLoginPassword": "s3cr3t-p@ss", + "sku": "Standard", + "ignoredNil": nil, + } + secureKeys := map[string]bool{"administratorLoginPassword": true} + + tfconfig.SetModuleParams(testRecipeName, params, secureKeys) + + secureVar := "radius_secret_redis_azure_administratorLoginPassword" + mod := tfconfig.Module[testRecipeName] + + // Non-secure params are set inline. + require.Equal(t, "admin", mod["administratorLogin"]) + require.Equal(t, "Standard", mod["sku"]) + // Nil params are skipped so a module variable default is not overridden with null. + _, hasNil := mod["ignoredNil"] + require.False(t, hasNil) + // The secure param references the sensitive variable instead of carrying the value inline. + require.Equal(t, "${var."+secureVar+"}", mod["administratorLoginPassword"]) + require.Equal(t, map[string]any{ + secureVar: map[string]any{ + "type": "string", + "sensitive": true, + "default": "s3cr3t-p@ss", + }, + }, tfconfig.Variable) + }) + + t.Run("unknown module name is a no-op", func(t *testing.T) { + tfconfig, err := New(context.Background(), testRecipeName, &envRecipe, &resourceRecipe) + require.NoError(t, err) + + tfconfig.SetModuleParams("does-not-exist", RecipeParams{"foo": "bar"}, nil) + + require.Nil(t, tfconfig.Variable) + _, hasFoo := tfconfig.Module[testRecipeName]["foo"] + require.False(t, hasFoo) + }) +} + func Test_AddAllOutputs(t *testing.T) { envRecipe, resourceRecipe := getTestInputs() diff --git a/pkg/recipes/terraform/config/types.go b/pkg/recipes/terraform/config/types.go index aad1a3f7cc6..fa97a9f1d07 100644 --- a/pkg/recipes/terraform/config/types.go +++ b/pkg/recipes/terraform/config/types.go @@ -86,6 +86,12 @@ type TerraformConfig struct { // https://developer.hashicorp.com/terraform/language/modules/syntax Module map[string]TFModuleConfig `json:"module"` + // Variable is the Terraform input variable configuration. Radius declares sensitive variables to + // carry secret-sourced module parameters so their values are redacted from Terraform plan/apply + // output. The value is a map keyed by variable name. + // https://developer.hashicorp.com/terraform/language/values/variables + Variable map[string]any `json:"variable,omitempty"` + // Output is the Terraform output configuration. // https://developer.hashicorp.com/terraform/language/values/outputs Output map[string]any `json:"output,omitempty"` diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index e14584a903a..4e3ab5ec9d8 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -371,6 +371,7 @@ func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, opt //update the recipe context with connected resources properties if options.ResourceRecipe != nil { recipectx.Resource.Connections = options.ResourceRecipe.ConnectedResourcesProperties + recipectx.Resource.Secrets = options.ResourceRecipe.Secrets } if err = tfConfig.AddRecipeContext(ctx, options.EnvRecipe.Name, recipectx); err != nil { @@ -388,6 +389,7 @@ func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, opt if options.ResourceRecipe != nil { recipectx.Resource.Connections = options.ResourceRecipe.ConnectedResourcesProperties + recipectx.Resource.Secrets = options.ResourceRecipe.Secrets } // Merge environment-level and resource-level parameters (resource parameters take precedence), @@ -397,9 +399,14 @@ func (e *executor) generateConfig(ctx context.Context, tf *tfexec.Terraform, opt resourceParams = options.ResourceRecipe.Parameters } mergedParams := recipes_util.ShallowMergeParameters(options.EnvRecipe.Parameters, resourceParams) - resolvedParams := paramresolver.ResolveParameterExpressions(mergedParams, recipectx) - if resolvedParams != nil { - tfConfig.Module[options.EnvRecipe.Name].SetParams(config.RecipeParams(resolvedParams)) + resolved, err := paramresolver.ResolveParameterExpressions(mergedParams, recipectx) + if err != nil { + return "", err + } + if resolved.Values != nil { + // Secret-sourced parameters (resolved.SecureKeys) are routed through sensitive Terraform + // variables so their values are redacted from plan/apply output. + tfConfig.SetModuleParams(options.EnvRecipe.Name, config.RecipeParams(resolved.Values), resolved.SecureKeys) } } if loadedModule.ResultOutputExists { diff --git a/pkg/recipes/types.go b/pkg/recipes/types.go index f21a751696f..236a7ca28b9 100644 --- a/pkg/recipes/types.go +++ b/pkg/recipes/types.go @@ -34,6 +34,11 @@ type ConnectedResource struct { Type string `json:"type"` // Properties represents the resource properties Properties map[string]any `json:"properties,omitempty"` + // Secrets holds resolved secret material for secret-typed connected resources, keyed by secret key. + // It is populated only from the secret store (never from Properties) and is tagged json:"-" so it is + // never serialized into the recipe context, logs, or IaC state. Recipe authors reference it via the + // context.resource.connections..secrets. expression path. + Secrets map[string]string `json:"-"` } // Configuration represents runtime and cloud provider configuration, which is used by the driver while deploying recipes. @@ -86,6 +91,14 @@ type EnvironmentDefinition struct { // Outputs maps resource property names to module output names for direct module support. // When nil or empty, all module outputs pass through with their original names. Outputs map[string]string + // SecretOutputs maps the name of a Radius.Security/secrets resource bound to the deploying resource + // (via an x-radius-secret-binding array property) to a map of that secret's data keys to module output + // names. After the recipe runs, the driver resolves each entry against the resource's bound secrets and + // the module's raw outputs into a RecipeOutput.SecretWriteBack; the resource controller then writes the + // values back into the bound secret (encrypted at rest + its backing Kubernetes Secret). This is the + // "backwards" secret flow for direct module support: a recipe's secure output populates a + // developer-authored secret rather than being exposed as a (redactable) resource property. + SecretOutputs map[string]map[string]string } // ResourceMetadata represents recipe details provided while deploying a portable or a user-defined resource. @@ -104,6 +117,21 @@ type ResourceMetadata struct { // The key is connection name and the value contains the connected resource's metadata and properties. // These are passed into the recipe context. ConnectedResourcesProperties map[string]ConnectedResource + // SecretReferences maps a resource property path marked with x-radius-secret-reference (e.g. "secretName") + // to the fully qualified Radius.Security/secrets resource ID that the property's value names. It is + // populated by the resource controller from the resource's schema and properties, and consumed by the + // engine to load the referenced secret's data into Secrets. + SecretReferences map[string]string + // SecretBindings lists the Radius.Security/secrets resource IDs declared by a resource's + // x-radius-secret-binding array property. It is populated by the resource controller from the resource's + // schema and properties, and consumed by the engine to load every key of each secret into Secrets under a + // "." namespace for the context.resource.secrets.. expression path. + SecretBindings []string + // Secrets holds resolved secret material for secret-reference properties, keyed by secret key. It is + // populated only from the referenced secret's deployed material (never from Properties) and is tagged + // json:"-" so it is never serialized into the recipe context, logs, or IaC state. Recipe authors + // reference it via the context.resource.secrets. expression path. + Secrets map[string]string `json:"-"` // Parameters represents key/value pairs to pass into the recipe template. Overrides any parameters set by the environment. Parameters map[string]any } @@ -133,6 +161,22 @@ type RecipeOutput struct { // Status represents the recipe status at deployment time of resource. Status *rpv1.RecipeStatus + + // SecretWriteBacks lists resolved write-backs of secure module outputs into developer-authored + // Radius.Security/secrets resources bound to the deploying resource (via the definition's SecretOutputs + // mapping). The driver populates it; the resource controller applies each one, patching the bound + // secret's data (encrypted at rest) and refreshing its backing Kubernetes Secret. + SecretWriteBacks []SecretWriteBack +} + +// SecretWriteBack is a resolved instruction to write secure module output values into a +// developer-authored Radius.Security/secrets resource bound to the deploying resource. It is the +// "backwards" secret flow: a recipe's secure output populates a developer-authored secret. +type SecretWriteBack struct { + // SecretID is the fully qualified resource ID of the bound Radius.Security/secrets resource. + SecretID string + // Data maps the bound secret's data keys to the plaintext values to write under them. + Data map[string]string } // SecretData represents secrets data and includes secret type and a map of secret keys to their values. @@ -164,6 +208,10 @@ type RecipeDefinition struct { // Outputs maps resource property names to module output names for direct module support. // When nil or empty, all module outputs pass through with their original names. Outputs map[string]string + // SecretOutputs maps the name of a bound Radius.Security/secrets resource to a map of that secret's + // data keys to module output names, writing the recipe's secure outputs back into the developer-authored + // secret (the "backwards" secret flow). Used for direct module support. + SecretOutputs map[string]map[string]string // PlainHTTP connects to the source using HTTP (not-HTTPS) PlainHTTP bool } diff --git a/pkg/schema/annotations.go b/pkg/schema/annotations.go index 03527ea3d20..4cfd873e4e1 100644 --- a/pkg/schema/annotations.go +++ b/pkg/schema/annotations.go @@ -50,6 +50,21 @@ func GetSensitiveFieldPaths(ctx context.Context, ucpClient *v20231001preview.Cli return ExtractSensitiveFieldPaths(schema, ""), nil } +// GetRetainFieldPaths fetches the schema for a resource and returns paths to fields marked with +// x-radius-retain. Retain fields are the subset of sensitive fields whose encrypted value is kept at +// rest (vault semantics) instead of being redacted after recipe execution. Mirrors GetSensitiveFieldPaths. +func GetRetainFieldPaths(ctx context.Context, ucpClient *v20231001preview.ClientFactory, resourceID string, resourceType string, apiVersion string) ([]string, error) { + schema, err := GetSchema(ctx, ucpClient, resourceID, resourceType, apiVersion) + if err != nil { + return nil, err + } + if schema == nil { + return nil, nil + } + + return ExtractRetainFieldPaths(schema, ""), nil +} + // GetSchema fetches the OpenAPI schema for a resource type and api version. // Returns nil if the schema is not found or the client is nil. func GetSchema(ctx context.Context, ucpClient *v20231001preview.ClientFactory, resourceID string, resourceType string, apiVersion string) (map[string]any, error) { @@ -82,6 +97,22 @@ func GetSchema(ctx context.Context, ucpClient *v20231001preview.ClientFactory, r // Supports object properties, array items, and additionalProperties (maps). // If a field is marked sensitive, its nested properties are not checked since the entire field is considered sensitive. func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { + return extractAnnotatedFieldPaths(schema, prefix, annotationRadiusSensitive) +} + +// ExtractRetainFieldPaths recursively walks the schema and returns paths to fields marked with +// x-radius-retain. Retain fields are the subset of sensitive fields whose encrypted value is kept at +// rest (vault semantics) instead of being redacted after recipe execution. The traversal mirrors +// ExtractSensitiveFieldPaths. +func ExtractRetainFieldPaths(schema map[string]any, prefix string) []string { + return extractAnnotatedFieldPaths(schema, prefix, annotationRadiusRetain) +} + +// extractAnnotatedFieldPaths recursively walks the schema and returns paths to fields where the given +// boolean annotation is set to true. The prefix parameter builds up the path as we traverse nested +// objects. Supports object properties, array items, and additionalProperties (maps). If a field is +// annotated, its nested properties are not checked since the entire field is treated as a leaf. +func extractAnnotatedFieldPaths(schema map[string]any, prefix string, annotation string) []string { var paths []string properties, ok := schema["properties"].(map[string]any) @@ -103,9 +134,9 @@ func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { fullPath = prefix + "." + fieldName } - // Check if this field has the x-radius-sensitive annotation - // If sensitive, treat the whole field as sensitive and skip nested properties. - if isSensitive, ok := fieldSchemaMap[annotationRadiusSensitive].(bool); ok && isSensitive { + // Check if this field has the annotation + // If annotated, treat the whole field as a leaf and skip nested properties. + if isAnnotated, ok := fieldSchemaMap[annotation].(bool); ok && isAnnotated { paths = append(paths, fullPath) continue } @@ -113,7 +144,7 @@ func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { // Recursively check nested objects if nestedProps, ok := fieldSchemaMap["properties"].(map[string]any); ok { nestedSchema := map[string]any{"properties": nestedProps} - nestedPaths := ExtractSensitiveFieldPaths(nestedSchema, fullPath) + nestedPaths := extractAnnotatedFieldPaths(nestedSchema, fullPath, annotation) paths = append(paths, nestedPaths...) } @@ -122,15 +153,15 @@ func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { if items, ok := fieldSchemaMap["items"].(map[string]any); ok { arrayItemPath := fullPath + "[*]" - // Check if items themselves are marked sensitive - // If sensitive, add the path and skip nested properties - if isSensitive, ok := items[annotationRadiusSensitive].(bool); ok && isSensitive { + // Check if items themselves are annotated + // If annotated, add the path and skip nested properties + if isAnnotated, ok := items[annotation].(bool); ok && isAnnotated { paths = append(paths, arrayItemPath) } else { // Recursively check nested properties within array items if itemProps, ok := items["properties"].(map[string]any); ok { itemSchema := map[string]any{"properties": itemProps} - nestedPaths := ExtractSensitiveFieldPaths(itemSchema, arrayItemPath) + nestedPaths := extractAnnotatedFieldPaths(itemSchema, arrayItemPath, annotation) paths = append(paths, nestedPaths...) } } @@ -141,15 +172,15 @@ func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { if additionalProps, ok := fieldSchemaMap["additionalProperties"].(map[string]any); ok { mapValuePath := fullPath + "[*]" - // Check if additionalProperties values are marked sensitive - // If sensitive, add the path and skip nested properties - if isSensitive, ok := additionalProps[annotationRadiusSensitive].(bool); ok && isSensitive { + // Check if additionalProperties values are annotated + // If annotated, add the path and skip nested properties + if isAnnotated, ok := additionalProps[annotation].(bool); ok && isAnnotated { paths = append(paths, mapValuePath) } else { // Recursively check nested properties within additionalProperties if addProps, ok := additionalProps["properties"].(map[string]any); ok { addPropsSchema := map[string]any{"properties": addProps} - nestedPaths := ExtractSensitiveFieldPaths(addPropsSchema, mapValuePath) + nestedPaths := extractAnnotatedFieldPaths(addPropsSchema, mapValuePath, annotation) paths = append(paths, nestedPaths...) } } @@ -159,7 +190,92 @@ func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { return paths } -// FieldPathSegment represents a single segment in a field path. +// ExtractSecretReferenceFieldPaths recursively walks the schema and returns paths to string fields marked +// with the x-radius-secret-reference annotation. Each such field holds the name of a Radius.Security/secrets +// resource. The prefix parameter builds up the path as we traverse nested objects. A marked field is treated +// as a leaf: its nested properties (if any) are not traversed. +func ExtractSecretReferenceFieldPaths(schema map[string]any, prefix string) []string { + var paths []string + + properties, ok := schema["properties"].(map[string]any) + if !ok { + return paths + } + + for fieldName, fieldSchema := range properties { + fieldSchemaMap, ok := fieldSchema.(map[string]any) + if !ok { + continue + } + + // Build the full path for this field. + var fullPath string + if prefix == "" { + fullPath = fieldName + } else { + fullPath = prefix + "." + fieldName + } + + // A field marked as a secret reference is treated as a leaf. + if isRef, ok := fieldSchemaMap[annotationRadiusSecretReference].(bool); ok && isRef { + paths = append(paths, fullPath) + continue + } + + // Recursively check nested objects. + if nestedProps, ok := fieldSchemaMap["properties"].(map[string]any); ok { + nestedSchema := map[string]any{"properties": nestedProps} + paths = append(paths, ExtractSecretReferenceFieldPaths(nestedSchema, fullPath)...) + } + } + + return paths +} + +// ExtractSecretBindingPaths recursively walks the schema and returns paths to properties marked with the +// x-radius-secret-binding annotation. Each such property is a "secrets binding": an array of Radius.Security/secrets +// resource IDs the resource depends on. The marked property is treated as a leaf: it is not traversed further, +// because the concrete secret IDs are read from the resource's properties at deploy time and every key of each +// secret is exposed to recipes under the context.resource.secrets.. path. The prefix parameter +// builds up the path as we traverse nested objects. +func ExtractSecretBindingPaths(schema map[string]any, prefix string) []string { + var paths []string + + properties, ok := schema["properties"].(map[string]any) + if !ok { + return paths + } + + for fieldName, fieldSchema := range properties { + fieldSchemaMap, ok := fieldSchema.(map[string]any) + if !ok { + continue + } + + // Build the full path for this field. + var fullPath string + if prefix == "" { + fullPath = fieldName + } else { + fullPath = prefix + "." + fieldName + } + + // A field marked as a secrets array is treated as a leaf. + if isBinding, ok := fieldSchemaMap[annotationRadiusSecretBinding].(bool); ok && isBinding { + paths = append(paths, fullPath) + continue + } + + // Recursively check nested objects. + if nestedProps, ok := fieldSchemaMap["properties"].(map[string]any); ok { + nestedSchema := map[string]any{"properties": nestedProps} + paths = append(paths, ExtractSecretBindingPaths(nestedSchema, fullPath)...) + } + } + + return paths +} + // A field path can contain field names, wildcards, and array indices. type FieldPathSegment struct { Type SegmentType diff --git a/pkg/schema/annotations_test.go b/pkg/schema/annotations_test.go index 1d7ee4d0998..81d9d7921e2 100644 --- a/pkg/schema/annotations_test.go +++ b/pkg/schema/annotations_test.go @@ -433,6 +433,276 @@ func TestExtractSensitiveFieldPaths_WithPrefix(t *testing.T) { require.Equal(t, []string{"parent.secret"}, result) } +func TestExtractRetainFieldPaths_WithPrefix(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "secret": map[string]any{ + "type": "string", + annotationRadiusSensitive: true, + annotationRadiusRetain: true, + }, + }, + } + + result := ExtractRetainFieldPaths(schema, "parent") + + require.Equal(t, []string{"parent.secret"}, result) +} + +func TestExtractRetainFieldPaths(t *testing.T) { + tests := []struct { + name string + schema map[string]any + expected []string + }{ + { + name: "empty schema", + schema: map[string]any{}, + expected: []string{}, + }, + { + name: "sensitive but not retained yields no retain paths", + schema: map[string]any{ + "properties": map[string]any{ + "password": map[string]any{ + "type": "string", + annotationRadiusSensitive: true, + }, + }, + }, + expected: []string{}, + }, + { + name: "single retained field", + schema: map[string]any{ + "properties": map[string]any{ + "name": map[string]any{ + "type": "string", + }, + "password": map[string]any{ + "type": "string", + annotationRadiusSensitive: true, + annotationRadiusRetain: true, + }, + }, + }, + expected: []string{"password"}, + }, + { + name: "only retained fields are returned", + schema: map[string]any{ + "properties": map[string]any{ + "password": map[string]any{ + "type": "string", + annotationRadiusSensitive: true, + annotationRadiusRetain: true, + }, + "apiKey": map[string]any{ + "type": "string", + annotationRadiusSensitive: true, + }, + }, + }, + expected: []string{"password"}, + }, + { + name: "retained map value (Radius.Security/secrets data shape)", + schema: map[string]any{ + "properties": map[string]any{ + "data": map[string]any{ + "type": "object", + "additionalProperties": map[string]any{ + "type": "object", + "properties": map[string]any{ + "value": map[string]any{ + "type": "string", + annotationRadiusSensitive: true, + annotationRadiusRetain: true, + }, + }, + }, + }, + }, + }, + expected: []string{"data[*].value"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ExtractRetainFieldPaths(tt.schema, "") + + // Sort both slices for comparison since map iteration order is not guaranteed + require.ElementsMatch(t, tt.expected, result) + }) + } +} + +func TestExtractSecretReferenceFieldPaths(t *testing.T) { + t.Run("top-level marked field", func(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "secretName": map[string]any{ + "type": "string", + annotationRadiusSecretReference: true, + }, + "host": map[string]any{ + "type": "string", + }, + }, + } + + require.Equal(t, []string{"secretName"}, ExtractSecretReferenceFieldPaths(schema, "")) + }) + + t.Run("nested marked field", func(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "config": map[string]any{ + "type": "object", + "properties": map[string]any{ + "secretName": map[string]any{ + "type": "string", + annotationRadiusSecretReference: true, + }, + }, + }, + }, + } + + require.Equal(t, []string{"config.secretName"}, ExtractSecretReferenceFieldPaths(schema, "")) + }) + + t.Run("marked field is treated as a leaf", func(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "secretName": map[string]any{ + "type": "object", + annotationRadiusSecretReference: true, + "properties": map[string]any{ + "nested": map[string]any{ + "type": "string", + annotationRadiusSecretReference: true, + }, + }, + }, + }, + } + + require.Equal(t, []string{"secretName"}, ExtractSecretReferenceFieldPaths(schema, "")) + }) + + t.Run("no marked fields", func(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "host": map[string]any{ + "type": "string", + }, + }, + } + + require.Empty(t, ExtractSecretReferenceFieldPaths(schema, "")) + }) + + t.Run("with prefix", func(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "secretName": map[string]any{ + "type": "string", + annotationRadiusSecretReference: true, + }, + }, + } + + require.Equal(t, []string{"parent.secretName"}, ExtractSecretReferenceFieldPaths(schema, "parent")) + }) +} + +func TestExtractSecretBindingPaths(t *testing.T) { + t.Run("top-level marked property", func(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "secrets": map[string]any{ + "type": "array", + annotationRadiusSecretBinding: true, + "items": map[string]any{"type": "string"}, + }, + "host": map[string]any{ + "type": "string", + }, + }, + } + + require.Equal(t, []string{"secrets"}, ExtractSecretBindingPaths(schema, "")) + }) + + t.Run("nested marked property", func(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "config": map[string]any{ + "type": "object", + "properties": map[string]any{ + "secrets": map[string]any{ + "type": "array", + annotationRadiusSecretBinding: true, + "items": map[string]any{"type": "string"}, + }, + }, + }, + }, + } + + require.Equal(t, []string{"config.secrets"}, ExtractSecretBindingPaths(schema, "")) + }) + + t.Run("marked property is treated as a leaf", func(t *testing.T) { + // A marked property is a leaf: the extractor must not descend into its sub-schema, even if that + // sub-schema contains another marked property. + schema := map[string]any{ + "properties": map[string]any{ + "secrets": map[string]any{ + "type": "array", + annotationRadiusSecretBinding: true, + "properties": map[string]any{ + "nested": map[string]any{ + "type": "array", + annotationRadiusSecretBinding: true, + }, + }, + }, + }, + } + + require.Equal(t, []string{"secrets"}, ExtractSecretBindingPaths(schema, "")) + }) + + t.Run("no marked properties", func(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "host": map[string]any{ + "type": "string", + }, + }, + } + + require.Empty(t, ExtractSecretBindingPaths(schema, "")) + }) + + t.Run("with prefix", func(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "secrets": map[string]any{ + "type": "array", + annotationRadiusSecretBinding: true, + "items": map[string]any{"type": "string"}, + }, + }, + } + + require.Equal(t, []string{"parent.secrets"}, ExtractSecretBindingPaths(schema, "parent")) + }) +} + func TestGetSensitiveFieldPaths(t *testing.T) { ctx := context.Background() diff --git a/pkg/schema/validator.go b/pkg/schema/validator.go index bc249e366b5..b8518ab9c26 100644 --- a/pkg/schema/validator.go +++ b/pkg/schema/validator.go @@ -40,6 +40,29 @@ const ( // Constants for annotation names const ( annotationRadiusSensitive = "x-radius-sensitive" + + // annotationRadiusRetain marks an x-radius-sensitive field whose encrypted value must be RETAINED + // at rest (vault semantics) instead of redacted to nil after recipe execution. Retained fields are + // persisted encrypted and redacted on read (GET/LIST) rather than on write, so the secrets loader + // can resolve the value by decrypting Radius's own stored copy instead of reading a cluster (required + // for multi-cluster). A retain field must also be x-radius-sensitive, otherwise its value would be + // persisted in plaintext (it would be neither encrypted on write nor redacted). + annotationRadiusRetain = "x-radius-retain" + + // annotationRadiusSecretReference marks a string property whose value is the name of a + // Radius.Security/secrets resource. Radius resolves the referenced secret's data and exposes it + // to recipes through the context.resource.secrets. expression path. Unlike x-radius-sensitive + // (which marks the secret value itself), this annotation marks a reference to a secret by name. + annotationRadiusSecretReference = "x-radius-secret-reference" + + // annotationRadiusSecretBinding marks an array property whose items are resource IDs of + // Radius.Security/secrets resources the resource depends on. Radius loads every key of each + // listed secret and exposes the values to recipes through the + // context.resource.secrets.. expression path, where is the secret + // resource's name and is a key within that secret's data. Unlike x-radius-secret-reference + // (a single secret named by a string property), this annotation binds a list of secrets by ID and + // namespaces their keys by the secret resource name. + annotationRadiusSecretBinding = "x-radius-secret-binding" ) // joinPath concatenates two path segments with a dot separator for property path tracking. @@ -349,6 +372,15 @@ func (v *Validator) validateRadiusConstraintsWithPath(schema *openapi3.Schema, p } } + // Check x-radius-retain annotation constraints + if err := v.checkRetainAnnotation(schema, path); err != nil { + if valErr, ok := err.(*ValidationError); ok { + errors.Add(valErr) + } else { + errors.Add(NewConstraintError("", err.Error())) + } + } + // Validate type constraints if err := v.validateTypeConstraints(schema, path); err != nil { if valErr, ok := err.(*ValidationError); ok { @@ -594,6 +626,40 @@ func (v *Validator) checkSensitiveAnnotation(schema *openapi3.Schema, path strin return nil } +// checkRetainAnnotation validates the x-radius-retain annotation. Retain means the encrypted value is +// kept at rest (vault semantics) instead of being redacted after recipe execution. A retain field must +// also be x-radius-sensitive; otherwise the value would be persisted in plaintext (neither encrypted on +// write nor redacted), which would be a security regression. +func (v *Validator) checkRetainAnnotation(schema *openapi3.Schema, path string) error { + if schema.Extensions == nil { + return nil + } + + retain, exists := schema.Extensions[annotationRadiusRetain] + if !exists { + return nil + } + + // Validate that the value is a boolean + boolVal, ok := retain.(bool) + if !ok { + return NewConstraintError(path, fmt.Sprintf("%s must be a boolean value", annotationRadiusRetain)) + } + + if !boolVal { + return nil + } + + // A retain field must also be marked x-radius-sensitive. + sensitive, sensitiveExists := schema.Extensions[annotationRadiusSensitive] + sensitiveVal, sensitiveIsBool := sensitive.(bool) + if !sensitiveExists || !sensitiveIsBool || !sensitiveVal { + return NewConstraintError(path, fmt.Sprintf("%s requires %s to be true on the same field", annotationRadiusRetain, annotationRadiusSensitive)) + } + + return nil +} + // isInternalRef checks if a $ref is an internal reference within the same document func (v *Validator) isInternalRef(ref string) bool { // Internal references start with "#/" which means they reference within the same document diff --git a/pkg/schema/validator_test.go b/pkg/schema/validator_test.go index 29664b1f657..8846d22769e 100644 --- a/pkg/schema/validator_test.go +++ b/pkg/schema/validator_test.go @@ -2340,6 +2340,106 @@ func TestValidator_checkSensitiveAnnotation(t *testing.T) { } } +func TestValidator_checkRetainAnnotation(t *testing.T) { + validator := NewValidator() + + tests := []struct { + name string + schema *openapi3.Schema + path string + hasErr bool + errMsg string + }{ + { + name: "retain with sensitive on string type - valid", + schema: &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + Extensions: map[string]any{ + annotationRadiusSensitive: true, + annotationRadiusRetain: true, + }, + }, + path: "data.value", + hasErr: false, + }, + { + name: "retain without sensitive - invalid", + schema: &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + Extensions: map[string]any{ + annotationRadiusRetain: true, + }, + }, + path: "data.value", + hasErr: true, + errMsg: fmt.Sprintf("%s requires %s to be true on the same field", annotationRadiusRetain, annotationRadiusSensitive), + }, + { + name: "retain with sensitive set to false - invalid", + schema: &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + Extensions: map[string]any{ + annotationRadiusSensitive: false, + annotationRadiusRetain: true, + }, + }, + path: "data.value", + hasErr: true, + errMsg: fmt.Sprintf("%s requires %s to be true on the same field", annotationRadiusRetain, annotationRadiusSensitive), + }, + { + name: "retain set to false - valid", + schema: &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + Extensions: map[string]any{ + annotationRadiusRetain: false, + }, + }, + path: "data.value", + hasErr: false, + }, + { + name: "no retain annotation - valid", + schema: &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + Extensions: map[string]any{ + annotationRadiusSensitive: true, + }, + }, + path: "password", + hasErr: false, + }, + { + name: "retain with non-boolean value - invalid", + schema: &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + Extensions: map[string]any{ + annotationRadiusSensitive: true, + annotationRadiusRetain: "true", + }, + }, + path: "data.value", + hasErr: true, + errMsg: fmt.Sprintf("%s must be a boolean value", annotationRadiusRetain), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validator.checkRetainAnnotation(tt.schema, tt.path) + if tt.hasErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errMsg) + var constraintErr *ValidationError + require.ErrorAs(t, err, &constraintErr) + require.Equal(t, ErrorTypeConstraint, constraintErr.Type) + } else { + require.NoError(t, err) + } + }) + } +} + func TestValidator_ValidateSchema_WithSensitiveAnnotation(t *testing.T) { validator := NewValidator() ctx := context.Background() diff --git a/swagger/specification/radius/resource-manager/Radius.Core/preview/2025-08-01-preview/openapi.json b/swagger/specification/radius/resource-manager/Radius.Core/preview/2025-08-01-preview/openapi.json index c6af4683a7d..5d8a6f11cdb 100644 --- a/swagger/specification/radius/resource-manager/Radius.Core/preview/2025-08-01-preview/openapi.json +++ b/swagger/specification/radius/resource-manager/Radius.Core/preview/2025-08-01-preview/openapi.json @@ -2021,6 +2021,16 @@ "additionalProperties": { "type": "string" } + }, + "secretOutputs": { + "type": "object", + "description": "Map of resource type property names to a map of Kubernetes Secret data keys to module output names. Used for recipes that point directly at a Bicep or Terraform module to materialize sensitive module outputs into a Kubernetes Secret in the application's namespace. For each entry, Radius creates a Secret whose data keys are populated from the named module outputs, and sets the named resource property to the generated Secret's name so connected resources can reference it.", + "additionalProperties": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + } } }, "required": [ diff --git a/typespec/Radius.Core/recipePacks.tsp b/typespec/Radius.Core/recipePacks.tsp index bc939e7b0c2..c590cf7833f 100644 --- a/typespec/Radius.Core/recipePacks.tsp +++ b/typespec/Radius.Core/recipePacks.tsp @@ -71,6 +71,9 @@ model RecipeDefinition { @doc("Map of resource type property names to module output names. Used for recipes that point directly at a Bicep or Terraform module to map the module's outputs onto the resource's properties.") outputs?: Record; + + @doc("Map of resource type property names to a map of Kubernetes Secret data keys to module output names. Used for recipes that point directly at a Bicep or Terraform module to materialize sensitive module outputs into a Kubernetes Secret in the application's namespace. For each entry, Radius creates a Secret whose data keys are populated from the named module outputs, and sets the named resource property to the generated Secret's name so connected resources can reference it.") + secretOutputs?: Record>; } @doc("The type of recipe")