Merge pull request #783 from jetstack/chart-update

[VC-48429] Helm chart updates for encrypted secrets

Author: SgtCoDFish

cmd/agent_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -32,6 +33,10 @@ func TestOutputModes(t *testing.T) {
 	})
 
 	t.Run("machinehub", func(t *testing.T) {
+		if strings.ToLower(os.Getenv("ARK_LIVE_TEST")) != "true" {
+			t.Skip("set ARK_LIVE_TEST=true to run this test against the live service")
+			return
+		}
 		arktesting.SkipIfNoEnv(t)
 
 		t.Log("This test runs against a live service and has been known to flake. If you see timeout issues it's possible that the test is flaking and it could be unrelated to your changes.")

deploy/charts/disco-agent/README.md

@@ -91,6 +91,13 @@ kubectl logs deployments/disco-agent --namespace "${NAMESPACE}" --follow
 > ```
 
 This will set the replicaset count more information can be found here: https://kubernetes.io/docs/concepts/workloads/controllers/replicaset/
+#### **acceptTerms** ~ `bool`
+> Default value:
+> ```yaml
+> false
+> ```
+
+Must be set to indicate that you have read and accepted the CyberArk Terms of Service. If false, the helm chart will fail to install and will print a message with instructions on how to accept the TOS.
 #### **image.repository** ~ `string`
 > Default value:
 > ```yaml
@@ -298,10 +305,11 @@ This description will be associated with the data that the agent uploads to the
 #### **config.sendSecretValues** ~ `bool`
 > Default value:
 > ```yaml
-> false
+> true
 > ```
 
-Enable sending of Secret values to CyberArk in addition to metadata. Metadata is always sent, but the actual values of Secrets are not sent by default. When enabled, Secret data is encrypted using envelope encryption using a key managed by CyberArk. Default: false (but default will change to true for a future release)
+Enable sending of Secret values to CyberArk in addition to metadata. Metadata is always sent, but the actual values of Secrets are not sent by default. When enabled, Secret data is encrypted using envelope encryption using a key managed by CyberArk, fetched from the Discovery and Context service.  
+Default: true
 #### **authentication.secretName** ~ `string`
 > Default value:
 > ```yaml

deploy/charts/disco-agent/templates/deployment.yaml

@@ -1,3 +1,6 @@
+{{- if not .Values.acceptTerms }}
+  {{- fail "\n\n=================================================================\n                Terms & Conditions Notice\n=================================================================\n\nBefore installing this application, you must review and accept\nthe terms and conditions available at:\nhttps://www.cyberark.com/contract-terms/\n\nTo proceed with installation, you must indicate acceptance by\nsetting:\n\n  - In your values file: acceptTerms: true\n  or\n  - Via the Helm flag: --set acceptTerms=true\n\nBy continuing with the next command, you confirm that you have\nreviewed and accepted these terms and conditions.\n\n=================================================================\n" }}
+{{- end }}
 apiVersion: apps/v1
 kind: Deployment
 metadata:

deploy/charts/disco-agent/values.schema.json

@@ -3,6 +3,9 @@
     "helm-values": {
       "additionalProperties": false,
       "properties": {
+        "acceptTerms": {
+          "$ref": "#/$defs/helm-values.acceptTerms"
+        },
         "affinity": {
           "$ref": "#/$defs/helm-values.affinity"
         },
@@ -84,6 +87,11 @@
       },
       "type": "object"
     },
+    "helm-values.acceptTerms": {
+      "default": false,
+      "description": "Must be set to indicate that you have read and accepted the CyberArk Terms of Service. If false, the helm chart will fail to install and will print a message with instructions on how to accept the TOS.",
+      "type": "boolean"
+    },
     "helm-values.affinity": {
       "default": {},
       "type": "object"
@@ -152,8 +160,8 @@
       "type": "string"
     },
     "helm-values.config.sendSecretValues": {
-      "default": false,
-      "description": "Enable sending of Secret values to CyberArk in addition to metadata. Metadata is always sent, but the actual values of Secrets are not sent by default. When enabled, Secret data is encrypted using envelope encryption using a key managed by CyberArk. Default: false (but default will change to true for a future release)",
+      "default": true,
+      "description": "Enable sending of Secret values to CyberArk in addition to metadata. Metadata is always sent, but the actual values of Secrets are not sent by default. When enabled, Secret data is encrypted using envelope encryption using a key managed by CyberArk, fetched from the Discovery and Context service.\nDefault: true",
       "type": "boolean"
     },
     "helm-values.extraArgs": {

deploy/charts/disco-agent/values.yaml

@@ -5,6 +5,9 @@
 # This will set the replicaset count more information can be found here: https://kubernetes.io/docs/concepts/workloads/controllers/replicaset/
 replicaCount: 1
 
+# Must be set to indicate that you have read and accepted the CyberArk Terms of Service. If false, the helm chart will fail to install and will print a message with instructions on how to accept the TOS.
+acceptTerms: false
+
 # This sets the container image more information can be found here: https://kubernetes.io/docs/concepts/containers/images/
 image:
   repository: ""
@@ -157,9 +160,9 @@ config:
   # Enable sending of Secret values to CyberArk in addition to metadata.
   # Metadata is always sent, but the actual values of Secrets are not sent by default.
   # When enabled, Secret data is encrypted using envelope encryption using
-  # a key managed by CyberArk.
-  # Default: false (but default will change to true for a future release)
-  sendSecretValues: false
+  # a key managed by CyberArk, fetched from the Discovery and Context service.
+  # Default: true
+  sendSecretValues: true
 
 authentication:
   secretName: agent-credentials

hack/ark/test-e2e.sh

@@ -101,7 +101,6 @@ kubectl apply -f "${root_dir}/hack/ark/cluster-external-secret.yaml"
 
 # We use a non-existent tag and omit the `--version` flag, to work around a Helm
 # v4 bug. See: https://github.com/helm/helm/issues/31600
-# TODO: shouldn't need to set config.sendSecretValues because it will default to true in future
 helm upgrade agent "oci://${ARK_CHART}:NON_EXISTENT_TAG@${ARK_CHART_DIGEST}" \
      --install \
      --wait \
@@ -114,7 +113,7 @@ helm upgrade agent "oci://${ARK_CHART}:NON_EXISTENT_TAG@${ARK_CHART_DIGEST}" \
      --set config.clusterName="e2e-test-cluster" \
      --set config.clusterDescription="A temporary cluster for E2E testing. Contact @wallrj-cyberark." \
      --set config.period=60s \
-     --set config.sendSecretValues=true \
+     --set acceptTerms=true \
      --set-json "podLabels={\"disco-agent.cyberark.cloud/test-id\": \"${RANDOM}\"}"
 
 kubectl rollout status deployments/disco-agent --namespace "${NAMESPACE}"

internal/cyberark/client_test.go

@@ -2,6 +2,8 @@ package cyberark_test
 
 import (
 	"crypto/x509"
+	"os"
+	"strings"
 	"testing"
 
 	"github.com/jetstack/venafi-connection-lib/http_client"
@@ -63,6 +65,11 @@ func TestCyberArkClient_PutSnapshot_MockAPI(t *testing.T) {
 //	go test ./internal/cyberark \
 //	  -v -count 1 -run TestCyberArkClient_PutSnapshot_RealAPI -args -testing.v 6
 func TestCyberArkClient_PutSnapshot_RealAPI(t *testing.T) {
+	if strings.ToLower(os.Getenv("ARK_LIVE_TEST")) != "true" {
+		t.Skip("set ARK_LIVE_TEST=true to run this test against the live service")
+		return
+	}
+
 	arktesting.SkipIfNoEnv(t)
 
 	t.Log("This test runs against a live service and has been known to flake. If you see timeout issues it's possible that the test is flaking and it could be unrelated to your changes.")

internal/cyberark/dataupload/dataupload.go

@@ -176,17 +176,25 @@ func (c *CyberArkClient) PutSnapshot(ctx context.Context, snapshot Snapshot) err
 	return nil
 }
 
+const SigV4Support = "sigv4"
+
 // RetrievePresignedUploadURLRequest is the JSON body sent to the inventory API to request a presigned upload URL.
 type RetrievePresignedUploadURLRequest struct {
 	ClusterID string `json:"cluster_id"`
 	Checksum  string `json:"checksum_sha256"`
 
 	// AgentVersion is the v-prefixed version of the agent uploading the snapshot.
-	// Note that the backend relies on this version being v-prefixed semver.
+	// Note that some versions of the backend rely on this version being v-prefixed semver,
+	// but that requirement was dropped in favour of the SigV4Support field below.
 	AgentVersion string `json:"agent_version"`
 
 	// FileSize is the size of the data we'll upload in bytes
 	FileSize int64 `json:"file_size"`
+
+	// SignatureVersion allows the agent to specify which version of AWS's signature scheme it expects for the presigned URL.
+	// Older versions of the agent will not send this. All versions which support this field will unconditionally set it to the
+	// value of SigV4Support, so the backend can rely on this field being set.
+	SignatureVersion string `json:"signature_version"`
 }
 
 func (c *CyberArkClient) retrievePresignedUploadURL(ctx context.Context, checksum string, clusterID string, fileSize int64) (string, string, error) {
@@ -196,10 +204,11 @@ func (c *CyberArkClient) retrievePresignedUploadURL(ctx context.Context, checksu
 	}
 
 	request := RetrievePresignedUploadURLRequest{
-		ClusterID:    clusterID,
-		Checksum:     checksum,
-		AgentVersion: version.PreflightVersion,
-		FileSize:     fileSize,
+		ClusterID:        clusterID,
+		Checksum:         checksum,
+		AgentVersion:     version.PreflightVersion,
+		FileSize:         fileSize,
+		SignatureVersion: SigV4Support,
 	}
 
 	encodedBody := &bytes.Buffer{}

internal/cyberark/dataupload/mock.go

@@ -132,6 +132,11 @@ func (mds *mockDataUploadServer) handleSnapshotLinks(w http.ResponseWriter, r *h
 		return
 	}
 
+	if req.SignatureVersion != SigV4Support {
+		http.Error(w, fmt.Sprintf("post body does not set signature_version=%s", SigV4Support), http.StatusInternalServerError)
+		return
+	}
+
 	if req.AgentVersion != version.PreflightVersion {
 		http.Error(w, fmt.Sprintf("post body contains unexpected agent version: %s", req.AgentVersion), http.StatusInternalServerError)
 		return

internal/envelope/rsa/encryptor.go

@@ -53,7 +53,8 @@ func (e *Encryptor) Encrypt(ctx context.Context, data []byte) (*envelope.Encrypt
 	}
 
 	// Encrypt using RSA-OAEP-256 for key algorithm and A256GCM for content encryption
-	// TODO: in go1.26+, consider using secret.Do to wrap this call, since it will generate an AES key
+	// TODO: When standardised, consider using secret.Do to wrap this call, since it will generate an AES key
+	// (see https://pkg.go.dev/runtime/secret)
 	encrypted, err := jwe.Encrypt(
 		data,
 		jwe.WithKey(jwa.RSA_OAEP_256(), key.Key, jwe.WithPerRecipientHeaders(headers)),