Skip to content

Comments

Add backend, GET, LIST controller for secret redaction#11195

Open
sk593 wants to merge 15 commits intomainfrom
add-decryption
Open

Add backend, GET, LIST controller for secret redaction#11195
sk593 wants to merge 15 commits intomainfrom
add-decryption

Conversation

@sk593
Copy link
Contributor

@sk593 sk593 commented Feb 6, 2026

Description

This PR adds support for the backend controller, GET controller, and LIST controller for secret decryption and redaction

Type of change

  • This pull request fixes a bug in Radius and has an approved issue (issue link required).
  • This pull request adds or changes features of Radius and has an approved issue (issue link required).
  • This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional).

Fixes: #11093 and #11095

Contributor checklist

Please verify that the PR meets the following requirements, where applicable:

  • An overview of proposed schema changes is included in a linked GitHub issue.
    • Yes
    • Not applicable
  • A design document PR is created in the design-notes repository, if new APIs are being introduced.
    • Yes
    • Not applicable
  • The design document has been reviewed and approved by Radius maintainers/approvers.
    • Yes
    • Not applicable
  • A PR for the samples repository is created, if existing samples are affected by the changes in this PR.
    • Yes
    • Not applicable
  • A PR for the documentation repository is created, if the changes in this PR affect the documentation or any user facing updates are made.
    • Yes
    • Not applicable
  • A PR for the recipes repository is created, if existing recipes are affected by the changes in this PR.
    • Yes
    • Not applicable

@sk593 sk593 requested a deployment to external-contributor-approval February 6, 2026 17:11 — with GitHub Actions Waiting
@sk593 sk593 requested a deployment to external-contributor-approval February 6, 2026 17:16 — with GitHub Actions Waiting
@sk593 sk593 requested a deployment to external-contributor-approval February 9, 2026 17:03 — with GitHub Actions Waiting
@sk593 sk593 requested a deployment to external-contributor-approval February 9, 2026 17:13 — with GitHub Actions Waiting
@sk593 sk593 requested a deployment to external-contributor-approval February 9, 2026 17:13 — with GitHub Actions Waiting
@sk593 sk593 requested a deployment to external-contributor-approval February 11, 2026 05:43 — with GitHub Actions Waiting
@sk593 sk593 marked this pull request as ready for review February 11, 2026 05:44
@sk593 sk593 requested review from a team as code owners February 11, 2026 05:44
@sk593 sk593 requested a deployment to external-contributor-approval February 11, 2026 05:52 — with GitHub Actions Waiting
@sk593 sk593 requested a deployment to external-contributor-approval February 11, 2026 06:06 — with GitHub Actions Waiting

// parseRedactPath parses a field path like "credentials.password" or "secrets[*].value"
// into path segments for traversal.
func parseRedactPath(path string) []redactPathSegment {
Copy link
Contributor

@lakshmimsft lakshmimsft Feb 12, 2026

Choose a reason for hiding this comment

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

this seems similar to schema.parseSensitivePath() below and encryption.parseFieldPath().. split dot-separated paths with [*] wildcards. Can we consider a shared utility function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

if err != nil {
logger.Error(err, "Failed to fetch sensitive field paths for GET redaction",
"resourceType", resourceType, "apiVersion", apiVersion)
// Continue without redaction on error - don't fail the GET
Copy link
Contributor

Choose a reason for hiding this comment

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

if there was an error here we want to raise it. if there are no paths sensitiveFieldPaths will be empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

if provisioningState != v1.ProvisioningStateSucceeded && resource.Properties != nil {
resourceID := serviceCtx.ResourceID.String()
resourceType := serviceCtx.ResourceID.Type()
apiVersion := getResourceAPIVersion(serviceCtx.APIVersion, resource)
Copy link
Contributor

@lakshmimsft lakshmimsft Feb 12, 2026

Choose a reason for hiding this comment

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

we should not be updating the internalMetadata or using a fallbackAPIversion. ln~86-97 should be removed. schema should be retrieved for the apiVersion resource was created with.
apiVersion := resource.InternalMetadata.UpdatedAPIVersion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

}

// Cache sensitive field paths for this resource type (same for all items)
var sensitiveFieldPaths []string
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably cache this per apiversion ( use map[string][]string instead of []string) since a list of resources for the resourcetype could return resources belonging to multiple api versions and schemas can differ between them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Copilot AI review requested due to automatic review settings February 13, 2026 22:12
@github-actions
Copy link

github-actions bot commented Feb 17, 2026

Unit Tests

4 844 tests   4 841 ✅  7m 47s ⏱️
  330 suites      3 💤
    1 files        0 ❌

Results for commit 82bd1ad.

♻️ This comment has been updated with latest results.

// testGetUCPClientFactoryWithSensitiveFields returns a UCP client that provides
// a schema with a sensitive "password" field. Uses the shared helper from
// encryptionfilter_test.go via the same Go test package.
func testGetUCPClientFactoryWithSensitiveFields() (*v20231001preview.ClientFactory, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this wrapper function and the next one on ln205 testGetUCPClientFactoryWithError() necessary? the inner functions can be called directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@codecov
Copy link

codecov bot commented Feb 17, 2026

Codecov Report

❌ Patch coverage is 79.88338% with 69 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.15%. Comparing base (209adb7) to head (82bd1ad).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...urces/backend/controller/createorupdateresource.go 60.22% 19 Missing and 16 partials ⚠️
pkg/schema/annotations.go 89.89% 6 Missing and 4 partials ⚠️
pkg/dynamicrp/backend/service.go 0.00% 4 Missing ⚠️
pkg/dynamicrp/frontend/getresource.go 90.00% 2 Missing and 2 partials ⚠️
pkg/dynamicrp/frontend/listresources.go 93.65% 2 Missing and 2 partials ⚠️
pkg/crypto/encryption/sensitive.go 84.21% 2 Missing and 1 partial ⚠️
pkg/dynamicrp/frontend/routes.go 0.00% 3 Missing ⚠️
pkg/dynamicrp/api/dynamicresource_conversion.go 50.00% 1 Missing and 1 partial ⚠️
pkg/recipes/terraform/execute.go 0.00% 2 Missing ⚠️
...p/datamodel/converter/dynamicresource_converter.go 0.00% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11195      +/-   ##
==========================================
+ Coverage   50.98%   51.15%   +0.16%     
==========================================
  Files         679      682       +3     
  Lines       43174    43433     +259     
==========================================
+ Hits        22012    22217     +205     
- Misses      19040    19069      +29     
- Partials     2122     2147      +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sk593 sk593 requested a deployment to external-contributor-approval February 17, 2026 22:33 — with GitHub Actions Waiting
Signed-off-by: sk593 <shruthikumar@microsoft.com>
@sk593 sk593 requested a deployment to external-contributor-approval February 17, 2026 23:15 — with GitHub Actions Waiting
@sk593 sk593 temporarily deployed to external-contributor-approval February 17, 2026 23:15 — with GitHub Actions Inactive
lakshmimsft
lakshmimsft previously approved these changes Feb 17, 2026
@sk593 sk593 temporarily deployed to external-contributor-approval February 18, 2026 23:28 — with GitHub Actions Inactive
@sk593 sk593 requested a deployment to external-contributor-approval February 18, 2026 23:38 — with GitHub Actions Waiting
Signed-off-by: sk593 <shruthikumar@microsoft.com>
@sk593 sk593 requested a deployment to external-contributor-approval February 19, 2026 16:00 — with GitHub Actions Waiting
@sk593 sk593 temporarily deployed to external-contributor-approval February 19, 2026 17:39 — with GitHub Actions Inactive
// normalizeSensitiveFieldTypes normalizes the type constraint on sensitive string fields
// so that the OpenAPI validator accepts both the original plaintext string and the encrypted object form.
// Only string types need this adjustment — object types remain objects after encryption and pass validation.
// normalizeSensitiveFieldTypes normalizes the type constraints on sensitive fields
Copy link
Contributor

Choose a reason for hiding this comment

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

this is needed only for string types. we do not want to normalize for types unless necessary. pls clarfy why this and related functions was updated ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

synced offline. the addition is for object properties. the encrypted value might not always match the original schema so when the validator runs, it fails without normalization

@sk593 sk593 temporarily deployed to external-contributor-approval February 20, 2026 18:40 — with GitHub Actions Inactive
@sk593 sk593 temporarily deployed to external-contributor-approval February 20, 2026 19:00 — with GitHub Actions Inactive
@radius-functional-tests
Copy link

radius-functional-tests bot commented Feb 20, 2026

Radius functional test overview

🔍 Go to test action run

Click here to see the test run details
Name Value
Repository radius-project/radius
Commit ref 82bd1ad
Unique ID funcc785c7e393
Image tag pr-funcc785c7e393
  • gotestsum 1.13.0
  • KinD: v0.29.0
  • Dapr: 1.14.4
  • Azure KeyVault CSI driver: 1.4.2
  • Azure Workload identity webhook: 1.3.0
  • Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-funcc785c7e393
  • Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
  • applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-funcc785c7e393
  • dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:pr-funcc785c7e393
  • controller test image location: ghcr.io/radius-project/dev/controller:pr-funcc785c7e393
  • ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-funcc785c7e393
  • deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting corerp-cloud functional tests...
⌛ Starting ucp-cloud functional tests...
❌ Test tool installation for corerp-cloud failed. Please check the logs for more details
❌ Failed to install Radius for corerp-cloud functional test. Please check the logs for more details
❌ Test tool installation for ucp-cloud failed. Please check the logs for more details
❌ Failed to install Radius for ucp-cloud functional test. Please check the logs for more details
❌ corerp-cloud functional test failed. Please check the logs for more details
❌ ucp-cloud functional test failed. Please check the logs for more details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Frontend Encryption Updates - Get controller

2 participants