From 22cbdf78af1bd855de3e12a056cb9c38635821a0 Mon Sep 17 00:00:00 2001 From: Georgi Yanev Date: Wed, 22 Apr 2026 18:52:30 +0300 Subject: [PATCH] VC-52159: add certOwnership Helm flag to discovery-agent chart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- deploy/charts/discovery-agent/README.md | 7 +++++++ .../discovery-agent/templates/configmap.yaml | 3 +++ .../charts/discovery-agent/values.schema.json | 8 ++++++++ deploy/charts/discovery-agent/values.yaml | 5 +++++ pkg/agent/config.go | 16 ++++++++++++--- pkg/agent/config_test.go | 15 ++++++++++++++ pkg/agent/run.go | 1 + pkg/client/client.go | 5 +++++ pkg/client/client_ngts.go | 5 +++++ pkg/client/client_ngts_test.go | 20 +++++++++++++++---- 10 files changed, 78 insertions(+), 7 deletions(-) diff --git a/deploy/charts/discovery-agent/README.md b/deploy/charts/discovery-agent/README.md index 9808f4b4..2366a0fc 100644 --- a/deploy/charts/discovery-agent/README.md +++ b/deploy/charts/discovery-agent/README.md @@ -35,6 +35,13 @@ A short description of the cluster where the agent is deployed (optional). This description will be associated with the data that the agent uploads to the backend. +#### **config.claimableCerts** ~ `bool` +> Default value: +> ```yaml +> false +> ``` + +Whether discovered certs can be claimed by other tenants (optional). true = certs are left unassigned, available for any tenant to claim. false (default) = certs are owned by this cluster's tenant. #### **config.period** ~ `string` > Default value: > ```yaml diff --git a/deploy/charts/discovery-agent/templates/configmap.yaml b/deploy/charts/discovery-agent/templates/configmap.yaml index 63677cc3..6ab0c43d 100644 --- a/deploy/charts/discovery-agent/templates/configmap.yaml +++ b/deploy/charts/discovery-agent/templates/configmap.yaml @@ -9,6 +9,9 @@ data: config.yaml: |- cluster_name: {{ required "config.clusterName is required" .Values.config.clusterName | quote }} cluster_description: {{ .Values.config.clusterDescription | quote }} + {{- if .Values.config.claimableCerts }} + claimable_certs: true + {{- end }} period: {{ .Values.config.period | quote }} {{- with .Values.config.excludeAnnotationKeysRegex }} exclude-annotation-keys-regex: diff --git a/deploy/charts/discovery-agent/values.schema.json b/deploy/charts/discovery-agent/values.schema.json index 489328b7..4a38bdf6 100644 --- a/deploy/charts/discovery-agent/values.schema.json +++ b/deploy/charts/discovery-agent/values.schema.json @@ -94,6 +94,9 @@ "helm-values.config": { "additionalProperties": false, "properties": { + "claimableCerts": { + "$ref": "#/$defs/helm-values.config.claimableCerts" + }, "clientID": { "$ref": "#/$defs/helm-values.config.clientID" }, @@ -127,6 +130,11 @@ }, "type": "object" }, + "helm-values.config.claimableCerts": { + "default": false, + "description": "Whether discovered certs can be claimed by other tenants (optional). true = certs are left unassigned, available for any tenant to claim. false (default) = certs are owned by this cluster's tenant.", + "type": "boolean" + }, "helm-values.config.clientID": { "default": "", "description": "Deprecated: Client ID for the configured service account. The client ID should be provided in the \"clientID\" field of the authentication secret (see config.secretName). This field is provided for compatibility for users migrating from the \"venafi-kubernetes-agent\" chart.", diff --git a/deploy/charts/discovery-agent/values.yaml b/deploy/charts/discovery-agent/values.yaml index 50a99596..a1c2662b 100644 --- a/deploy/charts/discovery-agent/values.yaml +++ b/deploy/charts/discovery-agent/values.yaml @@ -19,6 +19,11 @@ config: # +docs:property clusterDescription: "" + # Whether discovered certs can be claimed by other tenants (optional). + # true = certs are left unassigned, available for any tenant to claim. + # false (default) = certs are owned by this cluster's tenant. + claimableCerts: false + # How often to push data to the remote server # +docs:property period: "0h1m0s" diff --git a/pkg/agent/config.go b/pkg/agent/config.go index 94c67300..487f18ad 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -55,9 +55,13 @@ type Config struct { ClusterName string `yaml:"cluster_name"` // ClusterDescription is a short description of the Kubernetes cluster where the // agent is running. - ClusterDescription string `yaml:"cluster_description"` - DataGatherers []DataGatherer `yaml:"data-gatherers"` - VenafiCloud *VenafiCloudConfig `yaml:"venafi-cloud,omitempty"` + ClusterDescription string `yaml:"cluster_description"` + // ClaimableCerts controls whether discovered certs can be claimed by other tenants. + // true = certs are left unassigned, available for any tenant to claim. + // false (default) = certs are owned by this cluster's tenant. + ClaimableCerts bool `yaml:"claimable_certs"` + DataGatherers []DataGatherer `yaml:"data-gatherers"` + VenafiCloud *VenafiCloudConfig `yaml:"venafi-cloud,omitempty"` // For testing purposes. InputPath string `yaml:"input-path"` @@ -419,6 +423,11 @@ type CombinedConfig struct { // the agent is running. ClusterDescription string + // ClaimableCerts controls whether discovered certs can be claimed by other tenants. + // true = certs are left unassigned, available for any tenant to claim. + // false (default) = certs are owned by this cluster's tenant. + ClaimableCerts bool + // VenafiCloudVenafiConnection mode only. VenConnName string VenConnNS string @@ -733,6 +742,7 @@ func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags) res.ClusterID = clusterID res.ClusterName = clusterName res.ClusterDescription = cfg.ClusterDescription + res.ClaimableCerts = cfg.ClaimableCerts } // Validation of `data-gatherers`. diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index a01fd946..8488982a 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -1081,9 +1081,24 @@ func Test_ValidateAndCombineConfig_NGTS(t *testing.T) { assert.Equal(t, "test-tsg-123", got.TSGID) assert.Equal(t, "test-cluster", got.ClusterName) assert.Equal(t, "Test NGTS cluster", got.ClusterDescription) + assert.Equal(t, false, got.ClaimableCerts) assert.IsType(t, &client.NGTSClient{}, cl) }) + t.Run("ngts: claimable_certs flows from config into CombinedConfig", func(t *testing.T) { + t.Setenv("POD_NAMESPACE", "venafi") + privKeyPath := withFile(t, fakePrivKeyPEM) + got, _, err := ValidateAndCombineConfig(discardLogs(), + withConfig(testutil.Undent(` + period: 1h + cluster_name: test-cluster + claimable_certs: true + `)), + withCmdLineFlags("--ngts", "--tsg-id", "test-tsg-123", "--client-id", "test-client-id", "--private-key-path", privKeyPath)) + require.NoError(t, err) + assert.Equal(t, true, got.ClaimableCerts) + }) + t.Run("ngts: valid configuration with custom server URL", func(t *testing.T) { t.Setenv("POD_NAMESPACE", "venafi") privKeyPath := withFile(t, fakePrivKeyPEM) diff --git a/pkg/agent/run.go b/pkg/agent/run.go index e8f1c234..fce00b01 100644 --- a/pkg/agent/run.go +++ b/pkg/agent/run.go @@ -459,6 +459,7 @@ func postData(ctx context.Context, config CombinedConfig, preflightClient client err := preflightClient.PostDataReadingsWithOptions(ctx, readings, client.Options{ ClusterName: config.ClusterName, ClusterDescription: config.ClusterDescription, + ClaimableCerts: config.ClaimableCerts, // orgID and clusterID are not required for Venafi Cloud auth OrgID: config.OrganizationID, ClusterID: config.ClusterID, diff --git a/pkg/client/client.go b/pkg/client/client.go index 210243f0..6c9fd16f 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -23,6 +23,11 @@ type ( // Used for Venafi Cloud and MachineHub mode. ClusterDescription string + + // ClaimableCerts controls whether discovered certs can be claimed by other tenants. + // true = certs are left unassigned, available for any tenant to claim. + // false (default) = certs are owned by this cluster's tenant. + ClaimableCerts bool } // The Client interface describes types that perform requests against the Jetstack Secure backend. diff --git a/pkg/client/client_ngts.go b/pkg/client/client_ngts.go index 145e33c2..d4e27785 100644 --- a/pkg/client/client_ngts.go +++ b/pkg/client/client_ngts.go @@ -247,6 +247,11 @@ func (c *NGTSClient) PostDataReadingsWithOptions(ctx context.Context, readings [ query.Add("description", base64.RawURLEncoding.EncodeToString([]byte(stripHTML.Sanitize(opts.ClusterDescription)))) } + if opts.ClaimableCerts { + // The TLSPK backend reads "certOwnership=unassigned" — this is the backend contract. + query.Add("certOwnership", "unassigned") + } + uploadURL.RawQuery = query.Encode() klog.FromContext(ctx).V(2).Info( diff --git a/pkg/client/client_ngts_test.go b/pkg/client/client_ngts_test.go index 2229ea52..b1718426 100644 --- a/pkg/client/client_ngts_test.go +++ b/pkg/client/client_ngts_test.go @@ -1,7 +1,6 @@ package client import ( - "context" "encoding/json" "fmt" "net/http" @@ -331,7 +330,7 @@ func TestNGTSClient_PostDataReadingsWithOptions(t *testing.T) { ClusterDescription: "Test cluster description", } - err = client.PostDataReadingsWithOptions(context.Background(), readings, opts) + err = client.PostDataReadingsWithOptions(t.Context(), readings, opts) require.NoError(t, err) // Verify the upload request @@ -339,12 +338,25 @@ func TestNGTSClient_PostDataReadingsWithOptions(t *testing.T) { assert.Equal(t, "/"+ngtsUploadEndpoint, receivedRequest.URL.Path) assert.Contains(t, receivedRequest.URL.RawQuery, "name=test-cluster") assert.Equal(t, "Bearer test-access-token", receivedRequest.Header.Get("Authorization")) + // certOwnership not set — must NOT appear in query + assert.NotContains(t, receivedRequest.URL.RawQuery, "certOwnership") // Verify the payload var payload api.DataReadingsPost err = json.Unmarshal(receivedBody, &payload) require.NoError(t, err) assert.Equal(t, 1, len(payload.DataReadings)) + + // Verify claimableCerts=true is included when set + t.Run("claimableCerts: true sends certOwnership=unassigned to backend", func(t *testing.T) { + optsUnassigned := Options{ + ClusterName: "test-cluster", + ClaimableCerts: true, + } + err = client.PostDataReadingsWithOptions(t.Context(), readings, optsUnassigned) + require.NoError(t, err) + assert.Contains(t, receivedRequest.URL.RawQuery, "certOwnership=unassigned") + }) } func TestNGTSClient_AuthenticationFlow(t *testing.T) { @@ -384,7 +396,7 @@ func TestNGTSClient_AuthenticationFlow(t *testing.T) { opts := Options{ClusterName: "test"} for range 3 { - err = client.PostDataReadingsWithOptions(context.Background(), readings, opts) + err = client.PostDataReadingsWithOptions(t.Context(), readings, opts) require.NoError(t, err) } @@ -448,7 +460,7 @@ func TestNGTSClient_ErrorHandling(t *testing.T) { readings := []*api.DataReading{{DataGatherer: "test", Data: &api.DynamicData{}}} opts := Options{ClusterName: "test"} - err = client.PostDataReadingsWithOptions(context.Background(), readings, opts) + err = client.PostDataReadingsWithOptions(t.Context(), readings, opts) require.Error(t, err) assert.Contains(t, err.Error(), tt.expectedErrMsg) })