[super-agent] Add tests to the pre-install job (#1484)

Co-authored-by: Paolo Gallina <paologallina1992@gmail.com>
newrelic · Sep 25, 2024 · 3a14768 · 3a14768
1 parent 77ae959
commit 3a14768
Show file tree

Hide file tree

Showing 14 changed files with 126 additions and 48 deletions.
diff --git a/charts/super-agent/Chart.lock b/charts/super-agent/Chart.lock
@@ -4,9 +4,9 @@ dependencies:
   version: 2.13.0
 - name: super-agent-deployment
   repository: ""
-  version: 0.0.24-beta
+  version: 0.0.25-beta
 - name: common-library
   repository: https://helm-charts.newrelic.com
   version: 1.3.0
-digest: sha256:b15bf716a97ac0a42a4908344bf7e145ea63d02e1902f67bfdbd2b16bf74b6f2
-generated: "2024-09-17T10:10:30.582583+02:00"
+digest: sha256:cccfd633cd28b02a4369ccc1ddfa0bc0b768f0b4fa26e3987148977b0d8e74a9
+generated: "2024-09-23T16:38:04.016037+02:00"
diff --git a/charts/super-agent/Chart.yaml b/charts/super-agent/Chart.yaml
@@ -3,15 +3,15 @@ name: super-agent
 description: Bootstraps New Relic' Super Agent
 
 type: application
-version: 0.0.20-beta
+version: 0.0.21-beta
 
 dependencies:
   - name: flux2
     repository: https://fluxcd-community.github.io/helm-charts
     version: 2.13.0
     condition: flux2.enabled
   - name: super-agent-deployment
-    version: 0.0.24-beta
+    version: 0.0.25-beta
     condition: super-agent-deployment.enabled
     # The following dependency is needed as sub-dependency of super-agent-deployment
   - name: common-library

diff --git a/charts/super-agent/README.md b/charts/super-agent/README.md
@@ -51,7 +51,7 @@ As of the creation of the chart, it has no particularities and this section can
 | super-agent-deployment.affinity | object | `{}` | Sets pod/node affinities. Can be configured also with `global.affinity` |
 | super-agent-deployment.cleanupManagedResources | bool | `true` | Enable the cleanup of super-agent managed resources when the chart is uninstalled. If disabled, agents and/or agent configurations managed by the super-agent will not be deleted when the chart is uninstalled. |
 | super-agent-deployment.cluster | string | `""` | Name of the Kubernetes cluster monitored. Can be configured also with `global.cluster`. |
-| super-agent-deployment.config.opamp.auth.organization_id | string | `""` | Organization ID where fleets will live. |
+| super-agent-deployment.config.opamp.auth.organizationId | string | `""` | Organization ID where fleets will live. |
 | super-agent-deployment.config.opamp.auth.secret.client_id.base64 | string | `nil` | In case `.config.auth.secret.create` is true, you can set these keys to set client ID directly as base64. This options is mutually exclusive with `plain`. |
 | super-agent-deployment.config.opamp.auth.secret.client_id.plain | string | `nil` | In case `.config.auth.secret.create` is true, you can set these keys to set client ID directly as plain text. This options is mutually exclusive with `base64`. |
 | super-agent-deployment.config.opamp.auth.secret.client_id.secret_key | string | `client_id` | Key inside the secret containing the client ID. |

diff --git a/charts/super-agent/charts/super-agent-deployment/Chart.yaml b/charts/super-agent/charts/super-agent-deployment/Chart.yaml
@@ -4,7 +4,7 @@ description: A Helm chart to install New Relic Super agent on Kubernetes
 
 type: application
 
-version: 0.0.24-beta
+version: 0.0.25-beta
 
 keywords:
   - newrelic

diff --git a/charts/super-agent/charts/super-agent-deployment/templates/_helpers.tpl b/charts/super-agent/charts/super-agent-deployment/templates/_helpers.tpl
@@ -29,9 +29,9 @@ open-telemetry:
       global:
         licenseKey: ${nr-env:NR_LICENSE_KEY}
         cluster: ${nr-env:NR_CLUSTER_NAME}
-        nrStaging: ${nr-env:NR_STAGING}
-        verboseLog: ${nr-env:NR_VERBOSE}
-        region: ${nr-env:NR_REGION}
+      {{- if include "newrelic.common.nrStaging" . }}
+        nrStaging: true
+      {{- end -}}
 {{- end -}}
 {{- end -}}
 
@@ -175,9 +175,9 @@ Return .Values.config.auth.organizationId and fails if it does not exists
 */ -}}
 {{- define "newrelic-super-agent.auth.organizationId" -}}
 {{- if (((.Values.config).opamp).auth).organizationId -}}
-  {{- .Values.config.auth.organizationId -}}
+  {{- .Values.config.opamp.auth.organizationId -}}
 {{- else -}}
-  {{- fail ".config.auth.organizationId is required." -}}
+  {{- fail ".config.opamp.auth.organizationId is required" -}}
 {{- end -}}
 {{- end -}}
 

diff --git a/charts/super-agent/charts/super-agent-deployment/templates/deployment-superagent.yaml b/charts/super-agent/charts/super-agent-deployment/templates/deployment-superagent.yaml
@@ -89,12 +89,6 @@ spec:
                   key: {{ include "newrelic.common.license.secretKeyName" . }}
             - name: NR_CLUSTER_NAME
               value: {{ include "newrelic.common.cluster" . }}
-            - name: NR_STAGING
-              value: {{ include "newrelic.common.nrStaging.value" . | quote }}
-            - name: NR_VERBOSE
-              value: {{ include "newrelic.common.verboseLog.valueAsBoolean" . | quote }}
-            - name: NR_REGION
-              value: {{ include "newrelic.common.region" . }}
           {{- /* ----- Variables used to send data downstream to subagents */}}
 
           {{- with .Values.extraEnv }}
@@ -117,8 +111,7 @@ spec:
               readOnly: false
             {{- if ((.Values.config).opamp).enabled }}
             - name: auth-secret-private-key
-              mountPath: "/etc/newrelic-super-agent/keys/from-secret.key"
-              subPath: {{ include "newrelic-super-agent.auth.secret.privateKey.key" . }}
+              mountPath: "/etc/newrelic-super-agent/keys"
               readOnly: true
             {{- end }}
             {{- with .Values.extraVolumeMounts }}
@@ -140,6 +133,9 @@ spec:
         - name: auth-secret-private-key
           secret:
             secretName: {{ include "newrelic-super-agent.auth.secret.name" . }}
+            items:
+              - key: {{ include "newrelic-super-agent.auth.secret.privateKey.key" . }}
+                path: from-secret.key
         {{- end }}
         {{- with .Values.extraVolumes }}
         {{- toYaml . | nindent 8 }}

diff --git a/...gent/charts/super-agent-deployment/templates/preinstall-job-register-system-identity.yaml b/...gent/charts/super-agent-deployment/templates/preinstall-job-register-system-identity.yaml
@@ -18,6 +18,13 @@ metadata:
   name: {{ include "newrelic.common.naming.truncateToDNSWithSuffix" (dict "name" (include "newrelic.common.naming.fullname" .) "suffix" "preinstall-user-key" ) }}
   namespace: {{ .Release.Namespace }}
 data:
+  {{- /*
+    This secret is needed duplicating the userKey because it is in a helm hook and has a hook lifetime.
+    Internal helpers to get the userKey do not fail in case it is empty so I need to test here if it is empty.
+   */}}
+  {{- if not (include "newrelic.common.userKey._userKey" .) }}
+    {{- fail "You must specify a userKey or a customUserKeySecretName containing it" -}}
+  {{- end }}
   {{ include "newrelic.common.userKey.secretKeyName" . }}: {{ include "newrelic.common.userKey._userKey" . | b64enc }}
 {{- end }}
 ---
@@ -58,17 +65,20 @@ spec:
               apk update
               apk add kubectl
 
+              echo Checking if the secret '{{ include "newrelic-super-agent.auth.secret.name" . }}' is already present in the cluster
               if kubectl get secret {{ include "newrelic-super-agent.auth.secret.name" . }}; then
                 echo System identity already exists. Exiting gracefully...
                 exit 0
               fi
 
+              echo Generating the Keys...
               apk add curl jq openssl
               TEMPORAL_FOLDER=gen-folder
               mkdir $TEMPORAL_FOLDER
               openssl genrsa -out "$TEMPORAL_FOLDER/key" 4096
               openssl rsa -in "$TEMPORAL_FOLDER/key" -pubout -out "$TEMPORAL_FOLDER/pub"
 
+              echo Key generated, creating the identity...
               for RETRY in 1 2 3; do
                 HTTP_CODE=$(echo '{ "query":
                     "mutation {
@@ -83,7 +93,7 @@ spec:
                     }"
                   }' | tr -d $'\n' | \
                   curl \
-                    -s -w "%{http_code}" \
+                    -w "%{http_code}" \
                     -H "Content-Type: application/json" \
                     -H "API-Key: $USER_KEY" \
                     -o "$TEMPORAL_FOLDER/response.json" \
@@ -96,16 +106,16 @@ spec:
                 echo "Error creating the new system identity. The API endpoint returned $HTTP_CODE. Retrying ($RETRY/3)..."
                 sleep 2
               done
-              # Retry mechanism failed. Exiting...
-              if [ $HTTP_CODE -ne 200 ]; then exit 1; fi
+              if [ $HTTP_CODE -ne 200 ]; then echo HTTP_CODE=$HTTP_CODE ;exit 1; fi
 
               ERROR_MESSAGE=$(jq -r '.errors[0].message // "NOERROR"' "$TEMPORAL_FOLDER/response.json")
               if [ "$ERROR_MESSAGE" != "NOERROR" ]; then
                 echo "Failed to create a New Relic System Identity for OpAMP communication authentication. Please verify that your User Key is valid and that your Account Organization has the necessary permissions to create a System Identity: $ERROR_MESSAGE"
                 exit 1
               fi
 
-              kubectl create secret generic --dry-run -o json \
+              echo Creating the secret '{{ include "newrelic-super-agent.auth.secret.name" . }}'...
+              kubectl create secret generic --dry-run=client -o json \
                 {{ include "newrelic-super-agent.auth.secret.name" . }} \
                 --from-literal=CLIENT_ID=$(jq -r '.data.systemIdentityCreate.clientId' "$TEMPORAL_FOLDER/response.json") \
                 --from-file="private_key=$TEMPORAL_FOLDER/key" | \
@@ -117,7 +127,7 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:
   annotations:
-    helm.sh/hook: pre-install
+    helm.sh/hook: pre-install,pre-upgrade
     helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded
     helm.sh/hook-weight: "-1010"
   labels:
@@ -137,7 +147,7 @@ apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
 metadata:
   annotations:
-    helm.sh/hook: pre-install
+    helm.sh/hook: pre-install,pre-upgrade
     helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded
     helm.sh/hook-weight: "-1009"
   labels:
@@ -160,7 +170,7 @@ apiVersion: v1
 kind: ServiceAccount
 metadata:
   annotations:
-    helm.sh/hook: pre-install
+    helm.sh/hook: pre-install,pre-upgrade
     helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded
     helm.sh/hook-weight: "-1010"
   {{- if include "newrelic.common.serviceAccount.annotations" . }}

diff --git a/charts/super-agent/charts/super-agent-deployment/templates/rbac.yaml b/charts/super-agent/charts/super-agent-deployment/templates/rbac.yaml
@@ -61,6 +61,13 @@ rules:
       - get
       - patch
       - update
+  - apiGroups: [ "" ]
+    resources: ["secrets"]
+    verbs:
+      - delete
+    resourceNames:
+    - {{ include "newrelic-super-agent.auth.secret.name" . }}
+
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding

diff --git a/charts/super-agent/charts/super-agent-deployment/templates/uninstall-job.yaml b/charts/super-agent/charts/super-agent-deployment/templates/uninstall-job.yaml
@@ -45,14 +45,9 @@ spec:
                 kubectl -n {{ $.Release.Namespace }} delete {{ $cr }} -l {{ $saResourcesLabelSelector }}
               fi
               {{ end }}
-        {{- if include "newrelic-super-agent.auth.secret.shouldRunJob" . }}
-        - name: delete-system-identity
-          image: bitnami/kubectl  # TODO: Pin version to the latest that we support.
-          command:
-            - bash
-          args:
-            - -c
-            - |
+      
+              {{- if include "newrelic-super-agent.auth.secret.shouldRunJob" . }}
+              # Delete the secrets created in the cluster
               kubectl -n {{ $.Release.Namespace }} delete secret {{ include "newrelic-super-agent.auth.secret.name" . }}
-        {{- end }}
+              {{- end }}
 {{- end -}}
diff --git a/charts/super-agent/charts/super-agent-deployment/tests/auth_secret_test.yaml b/charts/super-agent/charts/super-agent-deployment/tests/auth_secret_test.yaml
@@ -31,10 +31,9 @@ tests:
             - mountPath: /var/lib/newrelic-super-agent
               name: var-lib-newrelic-super-agent
               readOnly: false
-            - mountPath: /etc/newrelic-super-agent/keys/from-secret.key
+            - mountPath: /etc/newrelic-super-agent/keys
               name: auth-secret-private-key
               readOnly: true
-              subPath: private_key
       - template: templates/deployment-superagent.yaml
         equal:
           path: spec.template.spec.volumes
@@ -50,6 +49,9 @@ tests:
             - name: auth-secret-private-key
               secret:
                 secretName: my-release-super-agent-deployment-auth
+                items:
+                  - key: private_key
+                    path: from-secret.key
       - template: templates/secret-sa-auth.yaml
         equal:
           path: metadata.name

diff --git a/...gent/charts/super-agent-deployment/tests/deployment_superagent_subagent_configs_test.yaml b/...gent/charts/super-agent-deployment/tests/deployment_superagent_subagent_configs_test.yaml
@@ -39,10 +39,9 @@ tests:
             - mountPath: /var/lib/newrelic-super-agent
               name: var-lib-newrelic-super-agent
               readOnly: false
-            - mountPath: /etc/newrelic-super-agent/keys/from-secret.key
+            - mountPath: /etc/newrelic-super-agent/keys
               name: auth-secret-private-key
               readOnly: true
-              subPath: private_key
       - template: templates/deployment-superagent.yaml
         equal:
           path: spec.template.spec.volumes
@@ -58,3 +57,6 @@ tests:
             - name: auth-secret-private-key
               secret:
                 secretName: my-release-super-agent-deployment-auth
+                items:
+                  - key: private_key
+                    path: from-secret.key
diff --git a/charts/super-agent/charts/super-agent-deployment/tests/preinstall_job_test.yaml b/charts/super-agent/charts/super-agent-deployment/tests/preinstall_job_test.yaml
@@ -0,0 +1,69 @@
+suite: pre-install job template
+templates:
+  - templates/preinstall-job-register-system-identity.yaml
+release:
+  name: my-release
+  namespace: my-namespace
+set:
+  cluster: test
+  licenseKey: test
+tests:
+  - it: by default it fails with missing values
+    asserts:
+      - failedTemplate: 
+          errorMessage: You must specify a userKey or a customUserKeySecretName containing it
+
+  - it: if userKey is set, it should fail with missing organization id
+    set:
+      userKey: test
+    asserts:
+      - failedTemplate:
+          errorMessage: .config.opamp.auth.organizationId is required
+
+  - it: if organizationId is set, it should fail with missing userKey
+    set:
+      config:
+        opamp:
+          auth:
+            organizationId: test
+    asserts:
+      - failedTemplate:
+          errorMessage: You must specify a userKey or a customUserKeySecretName containing it
+
+  - it: with everything set, the job should template correctly.
+    set:
+      userKey: test
+      config:
+        opamp:
+          auth:
+            organizationId: test
+    asserts:
+      - hasDocuments:
+          count: 5  # Secret, job, and 3 RBAC manifests
+      - documentIndex: 1
+        isNotNullOrEmpty:
+          path: spec.template.spec.containers[0].args
+
+  - it: with a custom secret for userKey, the secret should not be created.
+    set:
+      customUserKeySecretName: test-secret
+      customUserKeySecretKey: test-key
+      config:
+        opamp:
+          auth:
+            organizationId: test
+    asserts:
+      - hasDocuments:
+          count: 4  # With everything rendered it should be 5
+      - documentIndex: 0
+        isNotNullOrEmpty:
+          path: spec.template.spec.containers[0].args
+      - documentIndex: 0
+        contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: USER_KEY
+            valueFrom:
+              secretKeyRef:
+                name: test-secret
+                key: test-key
diff --git a/charts/super-agent/ci/test-values.yaml b/charts/super-agent/ci/test-values.yaml
@@ -4,7 +4,7 @@ global:
 
 super-agent-deployment:
   config:
-    auth:
-      # There is no way to test the auth flow for now. Tests get stuck as the pre-install job cannot succeed to create a new system identity.
-      # Until we have a better idea or we are able to create a fake oauth server, we have to disable this.
+    # There is no way to test the auth flow for now. Tests get stuck as the pre-install job cannot succeed to create a new system identity.
+    # Until we have a better idea or we are able to create a fake oauth server, we have to disable opamp.
+    opamp:
       enabled: false
diff --git a/charts/super-agent/values.yaml b/charts/super-agent/values.yaml
@@ -24,7 +24,7 @@ super-agent-deployment:
   image:
     registry:
     repository: newrelic/newrelic-super-agent
-    tag: 0.22.0
+    tag: 0.23.0
     imagePullPolicy: IfNotPresent
     # -- The secrets that are needed to pull images from a custom registry.
     pullSecrets: []
@@ -133,9 +133,6 @@ super-agent-deployment:
     #       global:
     #         licenseKey: ${nr-env:NR_LICENSE_KEY}
     #         cluster: ${nr-env:NR_CLUSTER_NAME}
-    #         nrStaging: ${nr-env:NR_STAGING}
-    #         verboseLog: ${nr-env:NR_VERBOSE}
-    #         region: ${nr-env:NR_REGION}
     #         # you can set here modifications to the open telemetry chart
 
     opamp:
@@ -145,7 +142,7 @@ super-agent-deployment:
 
       auth:
         # -- Organization ID where fleets will live.
-        organization_id: ""
+        organizationId: ""
         secret:
           create: true
           # -- Name auth' secret provided by the user. If the creation of this secret is set to `true`, this is the same the secret