Merge pull request #794 from jetstack/VC-52159-ngts-cert-ownership-agent

VC-52159: add certOwnership Helm flag to discovery-agent chart

Author: SgtCoDFish

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

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:

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.",

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"

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`.

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)

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,

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.

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(

pkg/client/client_ngts_test.go

@@ -1,7 +1,6 @@
 package client
 
 import (
-	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -331,20 +330,33 @@ 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
 	assert.NotNil(t, receivedRequest)
 	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)
 		})