VC-52159: add certOwnership Helm flag to discovery-agent chart#794
Merged
SgtCoDFish merged 1 commit intomasterfrom Apr 23, 2026
Merged
VC-52159: add certOwnership Helm flag to discovery-agent chart#794SgtCoDFish merged 1 commit intomasterfrom
SgtCoDFish merged 1 commit intomasterfrom
Conversation
SgtCoDFish
requested changes
Apr 23, 2026
| ClusterName: "test-cluster", | ||
| CertOwnership: "unassigned", | ||
| } | ||
| err = client.PostDataReadingsWithOptions(context.Background(), readings, optsUnassigned) |
Contributor
There was a problem hiding this comment.
nitpick: t.Context() over context.Background()
SgtCoDFish
reviewed
Apr 23, 2026
Comment on lines
+427
to
+428
| // "unassigned" = certs stored with NULL sub_tsg_id, claimable by any sub-TSG. | ||
| // Empty (default) = TSG_OWNED: certs inherit the cluster's sub-TSG ID. |
Contributor
There was a problem hiding this comment.
suggestion: are we expecting this to gain more values in the future? seems like it could be simpler as a boolean?
Author
There was a problem hiding this comment.
Moved to boolean, change the name to reflect more cleanly what it does. fix the comments
George-Yanev
force-pushed
the
VC-52159-ngts-cert-ownership-agent
branch
4 times, most recently
from
12e3a20 to
9d58853
Compare
Add a certOwnership configuration field that flows from Helm values to the upload URL query parameter (?certOwnership=unassigned). When set, the TLSPK backend stores cert_ownership_mode='unassigned' on the cluster and leaves discovered certificates with NULL sub_tsg_id (claimable by any sub-TSG) rather than stamping them with the cluster's sub-TSG ID. Changes: - pkg/client/client.go: CertOwnership field on Options struct - pkg/client/client_ngts.go: include ?certOwnership= in upload URL when set - pkg/agent/config.go: CertOwnership in Config and CombinedConfig - pkg/agent/run.go: pass CertOwnership through to PostDataReadingsWithOptions - deploy/charts/discovery-agent/values.yaml: certOwnership optional field - deploy/charts/discovery-agent/templates/configmap.yaml: render cert_ownership - deploy/charts/discovery-agent/values.schema.json: schema entry for new field - Tests: client_ngts_test and config_test verify the field flows end-to-end Local replace directive added for venafi-connection-lib (private GitHub repo requires SAML SSO — points to local clone at development time).
George-Yanev
force-pushed
the
VC-52159-ngts-cert-ownership-agent
branch
from
9d58853 to
22cbdf7
Compare
SgtCoDFish
approved these changes
Apr 23, 2026
Contributor
SgtCoDFish
left a comment
SgtCoDFish
left a comment
There was a problem hiding this comment.
/lgtm
/approve
I'm not 100% sure about the name "claimableCerts" - I think there might be a more descriptive name for that, but I can't think of a better one. But I don't think it's worth blocking over, and if we ship that name it's not the end of the world.
Thanks for this!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a certOwnership configuration field that flows from Helm values to the upload URL query parameter (?certOwnership=unassigned). When set, the TLSPK backend stores cert_ownership_mode='unassigned' on the cluster and leaves discovered certificates with NULL sub_tsg_id (claimable by any sub-TSG) rather than stamping them with the cluster's sub-TSG ID.
Changes:
Local replace directive added for venafi-connection-lib (private GitHub repo requires SAML SSO — points to local clone at development time).