Change helm chart to use common-library (#383)

* add common library to chart's dependencies

* Refactor fullname

* refactor selectorLabels

* refactor labels

* refactor podLabels

* refactor cluster

* refactor license

* refactor service accounts

* refactor lowdatamode

* refactor staging

* refactor tolerations

* refactor affinity

* refactor nodeSelector

* refactor security context

* refactor newrelic.mode

* refactor custom attributes

* refactor agent configuration files

* refactor verboseLog

* refactor priorityClassName

* refactor dnsconfig

* refactor imagePullSecrets

* refactor image

* refactor hostNetwork

* bring registry back to the values

* change env and envFrom back to extraEnv and extraEnvFrom

* remove unneeded secure forward only flags

* remove ksm unneeded tolerations

* fix tolerations evaluation in a more meaningful way

* fixed privileged mode not being correctly configure in the kubelet

* fix enableProcessMetrics behavior

* remove privileged default to be explicit in the values

* use podLabels and let the common library deal with merging this

* get rid of tolerations hidden defaults

* get rid of affinity hidden defaults

* forgot ksm tolerations

* nit-picking issues

* update common library to fix labels issue

* template license secret hash only when it is rendered

* change hostNetwork behaviour

* typo fix

.github/ct.yaml

@@ -2,3 +2,6 @@
 # or `cl` will fail with a not-so-helpful error that says:
 # "Error linting charts: Error identifying charts to process: Error running process: exit status 128"
 target-branch: main
+
+chart-repos:
+  - newrelic=https://helm-charts.newrelic.com

.gitignore

@@ -1,9 +1,32 @@
-.DS_Store
 bin
-.idea
 
 nri-kubernetes.yaml
 nri-kubernetes.yml
 
 charts/internal/e2e-resources/Chart.lock
 charts/internal/e2e-resources/charts/
+
+# Downloaded chart dependencies
+**/charts/*.tgz
+
+# OSX trash
+.DS_Store
+
+# Files generated by JetBrains IDEs, e.g. IntelliJ IDEA
+.idea/
+*.iml
+
+# Vscode files
+.vscode
+
+# Emacs save files
+*~
+\#*\#
+.\#*
+
+# Vim-related files
+[._]*.s[a-w][a-z]
+[._]s[a-w][a-z]
+*.un~
+Session.vim
+.netrwhist

charts/newrelic-infrastructure/Chart.lock

@@ -0,0 +1,6 @@
+dependencies:
+- name: common-library
+  repository: https://helm-charts.newrelic.com
+  version: 0.17.2
+digest: sha256:505ecd9915ecd1c3a5e406e5b30876d032486ed745d6868fb37a688e9780265d
+generated: "2022-04-13T10:27:38.214041+02:00"

charts/newrelic-infrastructure/Chart.yaml

@@ -1,14 +1,21 @@
 apiVersion: v2
 name: newrelic-infrastructure
 description: A Helm chart to deploy the New Relic Kubernetes monitoring solution
-version: 3.2.0
-appVersion: 3.2.0
-kubeVersion: ">=1.16.0-0"
 home: https://docs.newrelic.com/docs/kubernetes-pixie/kubernetes-integration/get-started/introduction-kubernetes-integration/
+icon: https://newrelic.com/themes/custom/curio/assets/mediakit/NR_logo_Horizontal.svg
 sources:
   - https://github.com/newrelic/nri-kubernetes/
   - https://github.com/newrelic/helm-charts/tree/master/charts/newrelic-infrastructure
-icon: https://newrelic.com/themes/custom/curio/assets/mediakit/NR_logo_Horizontal.svg
+
+version: 3.3.0
+appVersion: 3.1.1
+kubeVersion: ">=1.16.0-0"
+
+dependencies:
+  - name: common-library
+    version: 0.17.2
+    repository: "https://helm-charts.newrelic.com"
+
 maintainers:
   - name: alvarocabanas
     url: https://github.com/alvarocabanas
@@ -22,6 +29,7 @@ maintainers:
     url: https://github.com/paologallinaharbur
   - name: roobre
     url: https://github.com/roobre
+
 keywords:
   - infrastructure
   - newrelic

charts/newrelic-infrastructure/ci/test-cplane-kind-deployment-values.yaml

@@ -12,7 +12,9 @@ common:
       http:
         timeout: 180s
 
-customAttributes: '{"new":"relic","loren":"ipsum"}'
+customAttributes:
+  new: relic
+  loren: ipsum
 
 # Disable KSM scraper as it is not enabled when testing this chart individually.
 ksm:

charts/newrelic-infrastructure/ci/test-values.yaml

@@ -12,7 +12,9 @@ common:
       http:
         timeout: 180s
 
-customAttributes: '{"new":"relic","loren":"ipsum"}'
+customAttributes:
+  new: relic
+  loren: ipsum
 
 # Disable KSM scraper as it is not enabled when testing this chart individually.
 ksm:

charts/newrelic-infrastructure/templates/NOTES.txt

@@ -20,17 +20,18 @@ You have specified ksm or kubelet integration components as not enabled.
 Those components are needed to have the full experience on NROne kubernetes explorer.
 {{- end }}
 
-{{- if and .Values.controlPlane.enabled (not (include "newrelic.controlPlane.hostNetwork" .)) }}
+{{- if and .Values.controlPlane.enabled (not (include "nriKubernetes.controlPlane.hostNetwork" .)) }}
 Warning:
 ========
 
 Most Control Plane components listen in the loopback address only, which is not reachable without `hostNetwork: true`.
 Control plane autodiscovery might not work as expected.
-You can enable hostNetwork for control plane scraper pods only by setting `controlPlane.unprivilegedHostNetwork: true`,
-or alternative disable control plane monitoring altogether with `controlPlane.enabled: false`.
+You can enable hostNetwork for control plane scraper pods only by setting `global.hotNetwork`, `hostNetwork` or
+`controlPlane.hostNetwork: true` (in order of granularity for all the helm charts, this chart or only the control plane metrics
+discovery or alternative disable control plane monitoring altogether with `controlPlane.enabled: false`.
 {{- end }}
 
-{{- if and (include "newrelic.fargate" .) .Values.kubelet.affinity.nodeAffinity }}
+{{- if and (include "newrelic.fargate" .) .Values.kubelet.affinity }}
 Warning:
 ========
 
@@ -39,7 +40,7 @@ nodeAffinity rules, so we couldn't automatically exclude the kubelet daemonSet f
 Fargate nodes. In order for the integration to work, you MUST manually exclude
 the daemonSet from Fargate nodes.
 
-Please make sure your custom .Values.kubelet.affinity.nodeAffinity achieve the same effect as:
+Please make sure your `values.yaml' contains a .kubelet.affinity.nodeAffinity that achieve the same effect as:
 
 affinity:
   nodeAffinity:
@@ -101,10 +102,6 @@ future. Please migrate your agent config to the new format in the `common.agentC
 {{ $errors = printf "%s\n\n%s" $errors (include "newrelic.compatibility.message.logFile" . ) }}
 {{- end }}
 
-{{- if .Values.tolerations }}
-{{ $errors = printf "%s\n\n%s" $errors (include "newrelic.compatibility.message.tolerations" . ) }}
-{{- end }}
-
 {{- if .Values.resources }}
 {{ $errors = printf "%s\n\n%s" $errors (include "newrelic.compatibility.message.resources" . ) }}
 {{- end }}

charts/newrelic-infrastructure/templates/_common_library_overrides.tpl

@@ -0,0 +1,43 @@
+{{- /*
+By default the common library uses .Chart.Name for creating the name.
+This chart's name is too long so we shorted to `nrk8s`
+*/ -}}
+{{- define "common.naming.chartnameOverride" -}}
+nrk8s
+{{- end -}}
+
+
+
+{{- /* Allow to change container defaults dynamically based if we are running in privileged mode or not */ -}}
+{{- define "common.securityContext.containerDefaults" -}}
+runAsUser: 1000
+runAsGroup: 2000
+allowPrivilegeEscalation: false
+readOnlyRootFilesystem: true
+{{- end -}}
+
+
+
+{{- /* Allow to change pod defaults dynamically based if we are running in privileged mode or not */ -}}
+{{- define "common.securityContext.podDefaults" -}}
+{{- end -}}
+
+
+
+{{- /* Add mode to each object create */ -}}
+{{- define "common.labels.overrides.addLabels" -}}
+{{- if ( include "common.privileged" . ) -}}
+mode: privileged
+{{- else -}}
+mode: unprivileged
+{{- end -}}
+{{- end -}}
+
+
+
+{{/*
+This function allows easily to overwrite custom attributes to the function "common.customAttributes"
+*/}}
+{{- define "common.customAttributes.overrideAttributes" -}}
+clusterName: {{ include "common.cluster" . }}
+{{- end }}

charts/newrelic-infrastructure/templates/_helpers.tpl

@@ -1,145 +1,3 @@
-{{/*
-Expand the name of the chart.
-*/}}
-{{- define "newrelic.name" -}}
-{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
-{{- end }}
-
-
-{{/*
-Create a default fully qualified app name.
-We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
-If release name contains chart name it will be used as a full name.
-*/}}
-{{- define "newrelic.fullname" -}}
-{{- if .Values.fullnameOverride }}
-{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
-{{- else }}
-{{- $name := default "nrk8s" .Values.nameOverride }}
-{{- if contains $name .Release.Name }}
-{{- .Release.Name | trunc 63 | trimSuffix "-" }}
-{{- else }}
-{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
-{{- end }}
-{{- end }}
-{{- end }}
-
-
-{{/* Generate mode label */}}
-{{- define "newrelic.mode" }}
-{{- if .Values.privileged -}}
-privileged
-{{- else -}}
-unprivileged
-{{- end }}
-{{- end -}}
-
-{{/* Selector labels */}}
-{{- define "newrelic.selectorLabels" -}}
-app.kubernetes.io/name: {{ include "newrelic.name" . }}
-app.kubernetes.io/instance: {{ .Release.Name }}
-{{- end }}
-
-{{/* Common labels */}}
-{{- define "newrelic.labels" -}}
-helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
-{{ include "newrelic.selectorLabels" . }}
-mode: {{ template "newrelic.mode" . }}
-{{- if .Chart.AppVersion }}
-app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
-{{- end }}
-app.kubernetes.io/managed-by: {{ .Release.Service }}
-{{- end }}
-
-
-{{/* Create the name of the service account to use */}}
-{{- define "newrelic.serviceAccountName" -}}
-{{- if .Values.serviceAccount.create }}
-{{- default (include "newrelic.fullname" .) .Values.serviceAccount.name }}
-{{- else }}
-{{- default "default" .Values.serviceAccount.name }}
-{{- end }}
-{{- end }}
-
-{{/*
-Return the cluster name
-*/}}
-{{- define "newrelic.cluster" -}}
-{{- if .Values.cluster -}}
-  {{- .Values.cluster -}}
-{{- else if .Values.global -}}
-  {{- if .Values.global.cluster -}}
-    {{- .Values.global.cluster -}}
-  {{- end -}}
-{{- end -}}
-{{- end -}}
-
-{{/*
-Return local licenseKey if set, global otherwise
-*/}}
-{{- define "newrelic.licenseKey" -}}
-{{- if .Values.licenseKey -}}
-  {{- .Values.licenseKey -}}
-{{- else if .Values.global -}}
-  {{- if .Values.global.licenseKey -}}
-    {{- .Values.global.licenseKey -}}
-  {{- end -}}
-{{- end -}}
-{{- end -}}
-
-{{/*
-Return the name of the secret holding the License Key
-*/}}
-{{- define "newrelic.licenseCustomSecretName" -}}
-{{- if .Values.customSecretName -}}
-  {{- .Values.customSecretName -}}
-{{- else if .Values.global -}}
-  {{- if .Values.global.customSecretName -}}
-    {{- .Values.global.customSecretName -}}
-  {{- end -}}
-{{- end -}}
-{{- end -}}
-
-{{/*
-Return the name of the secret holding the License Key
-*/}}
-{{- define "newrelic.licenseSecretName" -}}
-{{ include "newrelic.licenseCustomSecretName" . | default (printf "%s-license" (include "newrelic.fullname" . )) }}
-{{- end -}}
-
-{{/*
-Return the name key for the License Key inside the secret
-*/}}
-{{- define "newrelic.licenseCustomSecretKey" -}}
-{{- if .Values.customSecretLicenseKey -}}
-  {{- .Values.customSecretLicenseKey -}}
-{{- else if .Values.global -}}
-  {{- if .Values.global.customSecretLicenseKey }}
-    {{- .Values.global.customSecretLicenseKey -}}
-  {{- end -}}
-{{- end -}}
-{{- end -}}
-
-{{/*
-Return the name key for the License Key inside the secret
-*/}}
-{{- define "newrelic.licenseSecretKey" -}}
-{{ include "newrelic.licenseCustomSecretKey" . | default "licenseKey" }}
-{{- end -}}
-
-{{/*
-Returns nrStaging
-*/}}
-{{- define "newrelic.nrStaging" -}}
-{{- if .Values.nrStaging -}}
-  {{- .Values.nrStaging -}}
-{{- else if .Values.global -}}
-  {{- if .Values.global.nrStaging -}}
-    {{- .Values.global.nrStaging -}}
-  {{- end -}}
-{{- end -}}
-{{- end -}}
-
 {{/*
 Returns fargate
 */}}
@@ -153,30 +11,7 @@ Returns fargate
 {{- end -}}
 {{- end -}}
 
-{{/*
-Returns lowDataMode
-*/}}
-{{- define "newrelic.lowDataMode" -}}
-{{/* `get` will return "" (empty string) if value is not found, and the value otherwise, so we can type-assert with kindIs */}}
-{{- if (get .Values "lowDataMode" | kindIs "bool") -}}
-  {{- if .Values.lowDataMode -}}
-    {{/*
-        We want only to return when this is true, returning `false` here will template "false" (string) when doing
-        an `(include "newrelic-logging.lowDataMode" .)`, which is not an "empty string" so it is `true` if it is used
-        as an evaluation somewhere else.
-    */}}
-    {{- .Values.lowDataMode -}}
-  {{- end -}}
-{{- else -}}
-{{/* This allows us to use `$global` as an empty dict directly in case `Values.global` does not exists */}}
-{{- $global := index .Values "global" | default dict -}}
-{{- if get $global "lowDataMode" | kindIs "bool" -}}
-  {{- if $global.lowDataMode -}}
-    {{- $global.lowDataMode -}}
-  {{- end -}}
-{{- end -}}
-{{- end -}}
-{{- end -}}
+
 
 {{/*
 Returns the list of namespaces where secrets need to be accessed by the controlPlane integration to do mTLS Auth
@@ -218,23 +53,10 @@ Returns the list of namespaces where secrets need to be accessed by the controlP
 roleBindingNamespaces: {{- uniq $namespaceList | toYaml | nindent 0 }}
 {{- end -}}
 
-{{/*
-Returns Custom Attributes even if formatted as a json string
-*/}}
-{{- define "newrelic.customAttributesWithoutClusterName" -}}
-{{- if kindOf .Values.customAttributes | eq "string" -}}
-{{  .Values.customAttributes }}
-{{- else -}}
-{{ .Values.customAttributes | toJson }}
-{{- end -}}
-{{- end -}}
 
-{{- define "newrelic.customAttributes" -}}
-{{- merge (include "newrelic.customAttributesWithoutClusterName" . | fromJson) (dict "clusterName" (include "newrelic.cluster" .)) | toJson }}
-{{- end -}}
 
 {{- define "newrelic.integrationConfigDefaults" -}}
-{{- if include "newrelic.lowDataMode" . -}}
+{{- if include "common.lowDataMode" . -}}
 interval: 30s
 {{- else  -}}
 interval: 15s