Skip to content

Conversation

@wallrj
Copy link
Member

@wallrj wallrj commented Aug 20, 2024

Stacked on #552

Ref: VC-35373, VC-35374

  • Added Helm values to allow the VenafiConnection based authentication to be enabled (and the original Service Account key based auth to be disabled).
  • Added template options to the CRDs to allow the user to skip installing them if they have already been installed by another TLSPK component.
  • Added VenafiConnection related RBAC and service account (based on the files in approver-policy-enterprise).
  • Added a test script to setup venafi-kubernetes-agent with VenafiConnection based auth, in a Kind cluster. Using simple bearer token auth rather than full blown secretless / workload identity federation.

Testing

hack/e2e/test.sh

I ran the hack/e2e/test.sh script on my laptop and it successfully posted to venafi.cloud.

helm template diff

The default Helm chart output has not changed, which hopefully indicates that existing users should be able to upgrade without problems.

$ diff -u <(helm template oci://registry.venafi.cloud/charts/venafi-kubernetes-agent --version 0.1.49) <(helm template deploy/charts/venafi-kubernetes-agent/)
Pulled: registry.venafi.cloud/charts/venafi-kubernetes-agent:0.1.49
Digest: sha256:0cc0244761fbc8fb48b3c7a94e58c4f7611b24432481f1ecf95ab4aa399033b9

Here's how the rendered manifests differ with authentication.venafiConnection.enabled:

$ diff -u <(helm template oci://registry.venafi.cloud/charts/venafi-kubernetes-agent --version 0.1.49) <(helm template deploy/charts/venafi-kubernetes-agent/ --set authentication.venafiConnection.enabled=true)
Pulled: registry.venafi.cloud/charts/venafi-kubernetes-agent:0.1.49
Digest: sha256:0cc0244761fbc8fb48b3c7a94e58c4f7611b24432481f1ecf95ab4aa399033b9
--- /dev/fd/63  2024-08-22 12:10:12.876782520 +0100
+++ /dev/fd/62  2024-08-22 12:10:12.876782520 +0100
@@ -578,6 +578,45 @@
     name: venafi-kubernetes-agent-release-name
     namespace: default
 ---
+# Source: venafi-kubernetes-agent/templates/venafi-rbac.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: venafi-kubernetes-agent-impersonate-role
+  namespace: "default"
+  labels:
+    helm.sh/chart: venafi-kubernetes-agent-0.1.49
+    app.kubernetes.io/name: venafi-kubernetes-agent
+    app.kubernetes.io/instance: release-name
+    app.kubernetes.io/version: "v0.1.49"
+    app.kubernetes.io/managed-by: Helm
+rules:
+- apiGroups: [ "" ]
+  resources: [ "serviceaccounts" ]
+  verbs: [ "impersonate" ]
+  resourceNames: [ "venafi-connection" ]
+---
+# Source: venafi-kubernetes-agent/templates/venafi-rbac.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: venafi-kubernetes-agent-impersonate-rolebinding
+  namespace: "default"
+  labels:
+    helm.sh/chart: venafi-kubernetes-agent-0.1.49
+    app.kubernetes.io/name: venafi-kubernetes-agent
+    app.kubernetes.io/instance: release-name
+    app.kubernetes.io/version: "v0.1.49"
+    app.kubernetes.io/managed-by: Helm
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: venafi-kubernetes-agent-impersonate-role
+subjects:
+- kind: ServiceAccount
+  name: venafi-kubernetes-agent-release-name
+  namespace: "default"
+---
 # Source: venafi-kubernetes-agent/templates/deployment.yaml
 apiVersion: apps/v1
 kind: Deployment
@@ -619,8 +658,10 @@
             - "agent"
             - "-c"
             - "/etc/venafi/agent/config/config.yaml"
-            - "--client-id"
-            - ""
+            - --venafi-connection
+            - "venafi-components"
+            - --venafi-connection-namespace
+            - "venafi"
             - "-p"
             - "0h1m0s"
             - --venafi-cloud
@@ -635,9 +676,6 @@
             - name: config
               mountPath: "/etc/venafi/agent/config"
               readOnly: true
-            - name: credentials
-              mountPath: "/etc/venafi/agent/key"
-              readOnly: true
           ports:
             - containerPort: 8081
               name: http-metrics
@@ -646,7 +684,3 @@
           configMap:
             name: agent-config
             optional: false
-        - name: credentials
-          secret:
-            secretName: agent-credentials
-            optional: false

@maelvls
Copy link
Member

maelvls commented Aug 20, 2024

I successfully ran ./hack/e2e/test.sh on my machine. I had a bit of a problem because https://ven-tlspk.venafi.cloud/ doesn't have the right tier to access the private OCI registries, but I manually ran the commands with my creds from https://glow-in-the-dark.venafi.cloud/ and it worked.

It seems like Michael's instructions in https://gitlab.com/venafi/vaas/ua/clouddocs/-/merge_requests/1133 match the Helm values that you propose in your PR. I am happy with your proposed changes.

Thank you a lot for helping with this! 🙏🙏

maelvls
maelvls previously approved these changes Aug 20, 2024
@wallrj wallrj force-pushed the VC-35373-venafi-connection-helm-chart branch from 7790b80 to e35591d Compare August 20, 2024 17:17
@maelvls
Copy link
Member

maelvls commented Aug 20, 2024

I was wrong about the "crd_bases":

unable to install CRDs onto control plane: unable to read CRD files: error converting YAML to JSON: yaml: did not find expected node content

The CRDs under templates/ are full of Helm markup, so we need the "crd_bases" so that integration tests work.

Could you add the file that was under crd_bases? You can do that with the following commands:

gh pr checkout 556
git checkout -
git checkout step1-makefile-modules -- deploy/charts/venafi-kubernetes-agent/crd_bases/jetstack.io_venaficonnections.yaml

Comment on lines 197 to 204
# -- When set to false, the rendered output does not contain the
# VenafiConnection CRDs and RBAC. This is useful for when the
# Venafi Connection resources are already installed separately.
enabled: false
Copy link
Member

@maelvls maelvls Aug 20, 2024

Choose a reason for hiding this comment

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

People will probably want to install the CRDs separately but still use the venafi connection auth method. It looks like the helm chart doesn't allow you to do that at the moment, WDYT? I confused crds.enabled with venafiConnection.enabled. Never mind.

Copy link
Member

Choose a reason for hiding this comment

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

Atanas also asked whether we could enable the venafi connection auth by default (i.e., --venafi-connection is used by default, but maybe not the CRDs).

For example, imagine that a customer install Venafi Kubernetes Agent through the VMG/operator, the Helm installation should work regardless of whether they forgot to configure Venafi Kubernetes Agent. They will see in the logs that the last step is to create a venafi connection object.

Would that work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Partly done. I rearranged the Helm values.
There are now

  • venafiConnection.include: Like VEI this controls whether the VenafiConnection CRD and related service account and RBAC get installed. (default false).
  • config.venafiConnection.enabled: This controls whether --venafi-connection=venafi-components is added to the command line and if true disables the old mounted Service account mechanism. (default false).

@wallrj wallrj force-pushed the VC-35373-venafi-connection-helm-chart branch from e35591d to dd4ab58 Compare August 20, 2024 17:27
@wallrj wallrj force-pushed the VC-35373-venafi-connection-helm-chart branch 4 times, most recently from 0dd0cc3 to 7d24222 Compare August 21, 2024 11:33
@wallrj wallrj requested a review from maelvls August 21, 2024 11:37
@wallrj wallrj dismissed maelvls’s stale review August 21, 2024 13:03

I updated the Helm values to make the options for installing VenafiConnection and using VenafiConnection more intuitive and consistent with other TLSPK tools (like venafi-enhanced-issuer). I also updated the test script to use the new bare minium Helm values and applied your suggested MacOS compatible date command.


# -- Configure the Venafi Kubernetes Agent to load credentials using a
# VenafiConnection resource.
venafiConnection:
Copy link
Member

@maelvls maelvls Aug 22, 2024

Choose a reason for hiding this comment

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

Why is this under config? As far as I understand, the config value is passed as a YAML file using the flag --agent-config-file or -c for short:

$ go run . agent --help
The agent will periodically gather data for the configured data
        gatherers and send it to a remote backend for evaluation

Usage:
  preflight agent [flags]
  preflight agent [command]

Available Commands:
  info        print several internal parameters of the agent
  rbac        print the agent's minimal RBAC manifest

Flags:
  -c, --agent-config-file agent.yaml     Config file location, default is agent.yaml in the current working directory. (default "./agent.yaml")

This means that the configuration file that gets stored in the configmap agent-config will now contain the field venafiConnection. It doesn't have any impact because this field is skipped when the YAML file is parsed. But it looks a little weird since this bit of configuration doesn't configure anything in the agent.

Would it not make more sense to move this to outside of the config block in that case?

Another solution would be for me to implement the venafiConnection field in the config file so that the user can either use the flag --venafi-connection=name, or use the config file field venafiConnection.name. I could even drop the command line flag in that case.

WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why is this under config?

Because that's how it's currently documented:

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes.

I'll add this to the configuration API then, which means you will no longer have to pass --venafi-connnection and --venafi-connnection-namespace in the deployment. This should make much more sense this way.

Copy link
Member Author

Choose a reason for hiding this comment

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

This means that the configuration file that gets stored in the configmap agent-config will now contain the field venafiConnection

No. That's not how the config template works in this project. You're thinking of cert-manager I think:

Copy link
Member Author

Choose a reason for hiding this comment

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

@maelvls I've moved the values around to hopefully make them more intuitive.
The venafiConnection.enabled value is now in the authentication section.
And the venafiConnection.include value is now in the crds section.

So the documentation would look like:

Use the following Helm chart values to install the VenafiConnection CRDs and to enable VenafiConnection based authentication:

config:
  clusterName: venafi-kubernetes-agent-e2e
  clusterDescription: |
      A kind cluster used for testing the venafi-kubernetes-agent.

authentication:
  venafiConnection:
    enabled: true

crds:
  venafiConnection:
    include: true

Or use the following Helm install command:

helm install venafi-kubernetes-agent oci://registry.venafi.cloud/charts/venafi-kubernetes-agent \
  --create-namespace \
  --namespace "venafi" \
  --set config.clusterName="replace-with-your-cluster-name" \
  --set config.clusterDescription="replace-with-your-cluster-description" \
  --set authentication.venafiConnection.enabled=true \
  --set crds.venafiConnection.include=true

Copy link
Member

@maelvls maelvls Aug 22, 2024

Choose a reason for hiding this comment

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

My bad, I didn't realize that config wasn't directly translated into a file, and that it is used to template the agent-config configmap. Thanks for pointing that out.

The new structure you propose is simple and clear, I like it.

I'm OK with the fact that --set crds.venafiConnection.include=true is a bit different in venafi-enhanced-issuer and approver-policy-enterprise (it would be --set venafiConnection.include=true on these two projects).

Signed-off-by: Richard Wall <richard.wall@venafi.com>
@wallrj wallrj force-pushed the VC-35373-venafi-connection-helm-chart branch from 7d24222 to 11621c5 Compare August 22, 2024 11:06
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed all the installation documentation from the Helm chart README and instead defer to the website documentation.
Saves having to document the secretless auth here too.

@maelvls
Copy link
Member

maelvls commented Aug 22, 2024

I've re-run the script and it still works with the new structure in values.yaml. The script seems to have worked. The script returned with the exit code 141, though. Not sure why.

$ export VEN_API_KEY=$APIKEY VEN_OWNING_TEAM=$(curl --fail-with-body -sS https://api.venafi.cloud/v1/teams -H "tppl-api-key: $APIKEY" | jq '.teams[0].id' -r) OCI_BASE=ttl.sh/maelvls
$ ./hack/e2e/test.sh
~/code/jetstack/jetstack-secure ~/code/jetstack/jetstack-secure/hack/e2e
2024/08/22 13:58:14 Using base cgr.dev/chainguard/static:latest@sha256:791657dd88dea8c1f9d3779815429f9c681a9a2778fc66dac3fbf550e1f1d9c8 for github.com/jetstack/preflight
2024/08/22 13:58:14 Building github.com/jetstack/preflight for linux/amd64
2024/08/22 13:58:20 Publishing ttl.sh/maelvls/images/venafi-agent:v0.1.49-19-g11621c54d0b619
2024/08/22 13:58:22 mounted blob: sha256:50c2c4796b3d6fd72518badc070d1271a375c94857c5469b1b9380e43d2bc180
2024/08/22 13:58:25 pushed blob: sha256:d400f7676c038abdd3c93b890d8f53ce11e9ad6425056849b93ee196e8c3e7bb
2024/08/22 13:58:25 pushed blob: sha256:b9b678d5804b31f4c3c8d7b4fc83733c7d5026fa8bb0eacaad9dc8230ea49cec
2024/08/22 13:58:25 pushed blob: sha256:250c06f7c38e52dc77e5c7586c3e40280dc7ff9bb9007c396e06d96736cf8542
2024/08/22 13:58:26 pushed blob: sha256:07a8590cc2a36109717682aeac93e7d071dfa6cdc4c50ad370f8ada08778687b
2024/08/22 13:58:27 ttl.sh/maelvls/images/venafi-agent:sha256-e02cb526fc054f0a046f536c7de7aa867dd43a7fad2e18c5a5daf2e137d5ce9b.sbom: digest: sha256:2cdf8a95a902a0e05cdfb579d23472817e431d07e58e4739fe7e8bc68b1c7c96 size: 374
2024/08/22 13:58:27 Published SBOM ttl.sh/maelvls/images/venafi-agent:sha256-e02cb526fc054f0a046f536c7de7aa867dd43a7fad2e18c5a5daf2e137d5ce9b.sbom
2024/08/22 13:59:55 pushed blob: sha256:9aff5900a9448f4b820cc11839f014b1b038c3f7ac34bb441cb75a98a60b5393
2024/08/22 13:59:56 ttl.sh/maelvls/images/venafi-agent:v0.1.49-19-g11621c54d0b619: digest: sha256:e02cb526fc054f0a046f536c7de7aa867dd43a7fad2e18c5a5daf2e137d5ce9b size: 1246
2024/08/22 13:59:56 Published ttl.sh/maelvls/images/venafi-agent:v0.1.49-19-g11621c54d0b619@sha256:e02cb526fc054f0a046f536c7de7aa867dd43a7fad2e18c5a5daf2e137d5ce9b
ttl.sh/maelvls/images/venafi-agent:v0.1.49-19-g11621c54d0b619@sha256:e02cb526fc054f0a046f536c7de7aa867dd43a7fad2e18c5a5daf2e137d5ce9b
Successfully packaged chart and saved it to: /Users/mvalais/code/jetstack/jetstack-secure/venafi-kubernetes-agent-v0.1.49-19-g11621c54d0b619.tgz
Pushed: ttl.sh/maelvls/charts/venafi-kubernetes-agent:v0.1.49-19-g11621c54d0b619
Digest: sha256:98ae8aaccb3a662db3b39ed2c7bde100a7a8c3cd0546b44a8642bfef39f1c476
~/code/jetstack/jetstack-secure/hack/e2e
2024/08/22 14:00:01 docker -v
2024/08/22 14:00:02 docker info --format '{{json .}}'
2024/08/22 14:00:02 docker ps -a --filter label=io.x-k8s.kind.cluster=kind --format '{{.Names}}'
ERROR: failed to create cluster: node(s) already exist for a cluster with the name "kind"
Error from server (AlreadyExists): namespaces "venafi" already exists
NAME                       TYPE                             DATA   AGE
venafi-image-pull-secret   kubernetes.io/dockerconfigjson   1      42h
Error from server (AlreadyExists): namespaces "venafi-kubernetes-agent-e2e" already exists
NAME                                  TYPE     DATA   AGE
cached-venafi-agent-service-account   Opaque   2      42h
Applying releases count=1
No orphan releases to uninstall
{
  "header": {
    "alg": "ES256",
    "kid": "Cy0co04hD3VHr1ZpT-ZHSCMMIxcRuunHbtrci5D1SIM",
    "typ": "JWT"
  },
  "payload": {
    "aud": "api.venafi.cloud/v1/oauth/token/serviceaccount",
    "exp": 1724329824,
    "iat": 1724328025,
    "iss": "13427e8e-5f16-11ef-bff6-3e2631dd4b3e",
    "jti": "95d5ff89eff4f2c80949b0f872e1f63344466769abe7c1fc925591ab66b28156",
    "nbf": 1724328025,
    "sub": "13427e8e-5f16-11ef-bff6-3e2631dd4b3e"
  },
  "signature": "XXX"
}
{"access_token":"XXX","expires_in":900,"token_type":"bearer"}
venaficonnection.jetstack.io/venafi-components unchanged
role.rbac.authorization.k8s.io/get-venafi-credentials unchanged
rolebinding.rbac.authorization.k8s.io/application-team-1-secret-rolebinding unchanged
secret/venafi-credentials configured
2024/08/22 12:00:23 Preflight agent version: development ()
2024/08/22 12:00:23 Loaded config:
schedule: ""
period: 0s
endpoint:
    protocol: ""
    host: ""
    path: ""
server: https://api.venafi.cloud/
organization_id: ""
cluster_id: venafi-kubernetes-agent-e2e
cluster_description: |
    A kind cluster used for testing the venafi-kubernetes-agent.
data-gatherers:
    - kind: k8s-dynamic
      name: k8s/fireflyissuers
      data_path: ""
      config:
        kubeconfig: ""
        groupversionresource:
            group: firefly.venafi.com
            version: v1
            resource: issuers
        exclude-namespaces: []
        include-namespaces: []
input-path: ""
output-path: ""
venafi-cloud:
    uploader_id: "no"
    upload_path: /v1/tlspk/upload/clusterdata
vCert: 2024/08/22 12:00:23 Venafi Connection mode was specified, using Venafi Connection authentication.
vCert: 2024/08/22 12:00:23 ignoring venafi-cloud.upload_path. In Venafi Connection mode, this field is not needed.
vCert: 2024/08/22 12:00:23 ignoring venafi-cloud.uploader_id. In Venafi Connection mode, this field is not needed.
2024/08/22 12:00:23 Prometheus was enabled.
Running prometheus server on port :8081
2024/08/22 12:01:16 Posting data to: https://api.venafi.cloud/
2024/08/22 12:01:17 Data sent successfully.
2024/08/22 12:02:17 Posting data to: https://api.venafi.cloud/
2024/08/22 12:02:18 Data sent successfully.

@wallrj wallrj merged commit 9b0e619 into venconn Aug 22, 2024
@wallrj wallrj deleted the VC-35373-venafi-connection-helm-chart branch August 22, 2024 12:21
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.

2 participants