From 3a14768a23578da3069c3cbde40915c4bd8617f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Manuel=20=22Kang=22=20P=C3=A9rez?= Date: Wed, 25 Sep 2024 15:13:55 +0200 Subject: [PATCH] [super-agent] Add tests to the pre-install job (#1484) Co-authored-by: Paolo Gallina --- charts/super-agent/Chart.lock | 6 +- charts/super-agent/Chart.yaml | 4 +- charts/super-agent/README.md | 2 +- .../charts/super-agent-deployment/Chart.yaml | 2 +- .../templates/_helpers.tpl | 10 +-- .../templates/deployment-superagent.yaml | 12 ++-- ...einstall-job-register-system-identity.yaml | 24 +++++-- .../templates/rbac.yaml | 7 ++ .../templates/uninstall-job.yaml | 13 ++-- .../tests/auth_secret_test.yaml | 6 +- ...ment_superagent_subagent_configs_test.yaml | 6 +- .../tests/preinstall_job_test.yaml | 69 +++++++++++++++++++ charts/super-agent/ci/test-values.yaml | 6 +- charts/super-agent/values.yaml | 7 +- 14 files changed, 126 insertions(+), 48 deletions(-) create mode 100644 charts/super-agent/charts/super-agent-deployment/tests/preinstall_job_test.yaml diff --git a/charts/super-agent/Chart.lock b/charts/super-agent/Chart.lock index 7f169b618..fa8007141 100644 --- 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 index 8df400508..6861f9094 100644 --- a/charts/super-agent/Chart.yaml +++ b/charts/super-agent/Chart.yaml @@ -3,7 +3,7 @@ name: super-agent description: Bootstraps New Relic' Super Agent type: application -version: 0.0.20-beta +version: 0.0.21-beta dependencies: - name: flux2 @@ -11,7 +11,7 @@ dependencies: 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 index 356ecb3ba..5b18a9a09 100644 --- 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 index f55eaaec9..bf78195ac 100644 --- 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 index ede9ef6e9..c10f6ffb3 100644 --- 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 index 2def77735..da7154c61 100644 --- 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/charts/super-agent/charts/super-agent-deployment/templates/preinstall-job-register-system-identity.yaml b/charts/super-agent/charts/super-agent-deployment/templates/preinstall-job-register-system-identity.yaml index 68b6f86c3..8b1acd977 100644 --- a/charts/super-agent/charts/super-agent-deployment/templates/preinstall-job-register-system-identity.yaml +++ b/charts/super-agent/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,8 +106,7 @@ 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 @@ -105,7 +114,8 @@ spec: 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 index cd939c717..18ea14873 100644 --- 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 index dc2e47318..e63718c52 100644 --- 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 index 8a476f74f..64a708f3c 100644 --- 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/charts/super-agent/charts/super-agent-deployment/tests/deployment_superagent_subagent_configs_test.yaml b/charts/super-agent/charts/super-agent-deployment/tests/deployment_superagent_subagent_configs_test.yaml index fb5fec557..38768f56c 100644 --- a/charts/super-agent/charts/super-agent-deployment/tests/deployment_superagent_subagent_configs_test.yaml +++ b/charts/super-agent/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 new file mode 100644 index 000000000..2bc5820ad --- /dev/null +++ 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 index fb6384cd2..1adfd5d73 100644 --- 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 index 7dd851b14..6cc1b15b6 100644 --- 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