Fix infinite reconcile loop (#445)

* Fix infinite reconcile loop Fix a potential infinite reconcile loop when deploying Grafana due to updated hashing algorithm which changes the hash contents each time the contents is hashed. Adds a fix which results in skipping the update of the default-grafana-htpasswd Secret object to avoid the situation. Also makes a small add to the when clause list for deploying Elasticsearch so that only when it is enabled are the tasks included. Closes: STF-1499 Signed-off-by: Leif Madsen <[email protected]> * Logic change so Secret updates on parameter change Update the htpasswd secret when the ServiceTelemetry CRD is updated for a basicAuth password change. Will work with existing deployments, adding the annotation to an existing secret to get everything aligned. Signed-off-by: Leif Madsen <[email protected]> * Syntax and lint fix * Remove TODO for creating dashboards Remove TODO for creating dashboards as management of dashboards is outside the scope of STF. * Update grafana htpasswd management logic Update the logic for grafana htpasswd management to no longer use sha256 hashes and instead use bcrypt. Update the logic so that we create the initial contents of the secret before writing it to the first secret so that we can avoid a second Secret k8s_info call. Create the secret if it isn't already created. If it is created, then read the existing salt and generate a hash with it. Update the secret and the annotation for grafana deployment. If no changes are found, then the secret and the annotation will not be updated, resulting in no grafana restart. NOTE: this commit has debugging logic in it to show how it works. Following commit removes it and won't be available in future squashed history. See original pull-request. * Remove debug logic * Adjust Ansible play names --------- Signed-off-by: Leif Madsen <[email protected]>
infrawatch · Aug 25, 2023 · 0dfe245 · 0dfe245
1 parent f88fa19
commit 0dfe245
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 51 deletions.
diff --git a/roles/servicetelemetry/tasks/component_grafana.yml b/roles/servicetelemetry/tasks/component_grafana.yml
@@ -7,8 +7,43 @@
         kind: Route
         name: 'grafana-route'
 
-- name: Create htpasswd secret for grafana admin
+- name: Check for existing grafana htpasswd secret
   no_log: true
+  k8s_info:
+    api_version: v1
+    kind: Secret
+    namespace: '{{ ansible_operator_meta.namespace }}'
+    name: '{{ ansible_operator_meta.name }}-grafana-htpasswd'
+  register: grafana_htpasswd_secret
+
+- block:
+    - name: Parse current Grafana htpasswd salt from secret
+      no_log: true
+      set_fact:
+        grafana_htpasswd_salt: "{{ ((grafana_htpasswd_secret.resources[0].data.auth | b64decode).split('$')[-1])[0:22] }}"
+  rescue:
+    - name: Generate initial Grafana htpasswd bcrypt string from grafana.admin_password
+      no_log: true
+      set_fact:
+        init_grafana_htpasswd_bcrypt_string: "{{ (servicetelemetry_vars.graphing.grafana.admin_password | password_hash('bcrypt') | replace('$2b$','$2y$', 1)) }}"
+
+    - name: Read newly generated Grafana htpasswd salt
+      no_log: true
+      set_fact:
+        grafana_htpasswd_salt: "{{ (init_grafana_htpasswd_bcrypt_string.split('$')[-1])[0:22] }}"
+  always:
+    - name: Generate Grafana htpasswd bcrypt string from grafana.adminPassword using salt
+      no_log: true
+      set_fact:
+        grafana_htpasswd_bcrypt_string: "{{ (servicetelemetry_vars.graphing.grafana.admin_password | password_hash('bcrypt', grafana_htpasswd_salt) | replace('$2b$','$2y$', 1)) }}"
+
+    - name: Generate Grafana auth string from grafana.adminUser and grafana_htpasswd_bcrypt_string
+      no_log: true
+      set_fact:
+        grafana_htpasswd_auth_string: "{{ servicetelemetry_vars.graphing.grafana.admin_user }}:{{ grafana_htpasswd_bcrypt_string }}"
+
+- name: Create or patch htpasswd secret for grafana admin
+  no_log: false
   k8s:
     definition:
       api_version: v1
@@ -18,7 +53,7 @@
         namespace: '{{ ansible_operator_meta.namespace }}'
       type: Opaque
       stringData:
-        auth: '{{ servicetelemetry_vars.graphing.grafana.admin_user }}:{{ servicetelemetry_vars.graphing.grafana.admin_password | password_hash("bcrypt") | replace("$2b$","$2y$", 1) }}'
+        auth: '{{ grafana_htpasswd_auth_string }}'
 
 - name: Lookup template
   debug:
@@ -34,49 +69,49 @@
     state: '{{ "present" if servicetelemetry_vars.graphing.enabled else "absent" }}'
     definition:
       '{{ grafana_manifest }}'
-  when: servicetelemetry_vars.graphing.enabled
 
 - when: servicetelemetry_vars.graphing.enabled
   block:
-  - when: servicetelemetry_vars.backends.metrics.prometheus.enabled
-    block:
-    - name: Retrieve configmap for OAUTH CA certs
-      k8s_info:
-        api_version: v1
-        kind: ConfigMap
-        name: serving-certs-ca-bundle
-        namespace: '{{ ansible_operator_meta.namespace }}'
-      register: serving_certs_ca
+    - when: servicetelemetry_vars.backends.metrics.prometheus.enabled
+      block:
+        - name: Retrieve configmap for OAUTH CA certs
+          k8s_info:
+            api_version: v1
+            kind: ConfigMap
+            name: serving-certs-ca-bundle
+            namespace: '{{ ansible_operator_meta.namespace }}'
+          register: serving_certs_ca
 
-    - name: Retrieve prometheus secret
-      k8s_info:
-        api_version: v1
-        kind: Secret
-        namespace: '{{ ansible_operator_meta.namespace }}'
-        name: '{{ ansible_operator_meta.name }}-prometheus-htpasswd'
-      register: prometheus_secret
+        - name: Retrieve prometheus secret
+          k8s_info:
+            api_version: v1
+            kind: Secret
+            namespace: '{{ ansible_operator_meta.namespace }}'
+            name: '{{ ansible_operator_meta.name }}-prometheus-htpasswd'
+          register: prometheus_secret
 
-    - name: Decode prometheus password
-      no_log: true
-      set_fact:
-        prom_basicauth_passwd: '{{ prometheus_secret.resources[0].data.password | b64decode }}'
+        - name: Decode prometheus password
+          no_log: true
+          set_fact:
+            prom_basicauth_passwd: '{{ prometheus_secret.resources[0].data.password | b64decode }}'
 
-  # Lookup existing datasources
-  - name: Remove legacy datasources
-    k8s:
-      api_version: integreatly.org/v1alpha1
-      name: '{{ ansible_operator_meta.name }}-ds-prometheus'
-      kind: GrafanaDataSource
-      namespace: '{{ ansible_operator_meta.namespace }}'
-      state: absent
+    # Lookup existing datasources
+    - name: Remove legacy datasources
+      k8s:
+        api_version: integreatly.org/v1alpha1
+        name: '{{ ansible_operator_meta.name }}-ds-prometheus'
+        kind: GrafanaDataSource
+        namespace: '{{ ansible_operator_meta.namespace }}'
+        state: absent
 
-  - name: Set datasources
-    set_fact:
-      ds_manifest: "{{ lookup('template', './manifest_grafana_ds.j2') | from_yaml }}"
-    when: ds_manifest is not defined
+    # NOTE: this can fail if you enable grafana without prometheus due to missing resources referenced in the template
+    - name: Set datasources
+      set_fact:
+        ds_manifest: "{{ lookup('template', './manifest_grafana_ds.j2') | from_yaml }}"
+      when: ds_manifest is not defined
 
-  - name: Create the datasources
-    k8s:
-      state: '{{ "present" if servicetelemetry_vars.graphing.enabled else "absent" }}'
-      definition:
-        '{{ ds_manifest }}'
+    - name: Create the datasources
+      k8s:
+        state: '{{ "present" if servicetelemetry_vars.graphing.enabled else "absent" }}'
+        definition:
+          '{{ ds_manifest }}'
diff --git a/roles/servicetelemetry/tasks/main.yml b/roles/servicetelemetry/tasks/main.yml
@@ -71,6 +71,7 @@
     - has_elasticsearch_api | bool
     - has_certmanager_api | bool
     - observability_strategy in ['use_community', 'use_hybrid']
+    - servicetelemetry_vars.backends.events.elasticsearch.enabled | bool
 
 # --> backends.logs
 - name: Check if we have loki API
@@ -99,17 +100,11 @@
   set_fact:
     has_integreatly_api: "{{ True if 'integreatly.org' in api_groups else False }}"
 
-- name: Deploy graphing
-  block:
-  - name: Create Grafana instance
-    include_tasks: component_grafana.yml
-
-# TODO
-#  - name: Create dashboards
-#    include_tasks: component_dashboards.yml
-  when:
+- when:
     - has_integreatly_api | bool
     - observability_strategy in ['use_community', 'use_hybrid']
+  name: Start graphing component plays
+  include_tasks: component_grafana.yml
 
 # Post deployment tasks
 - name: Post-setup

diff --git a/roles/servicetelemetry/templates/manifest_grafana.j2 b/roles/servicetelemetry/templates/manifest_grafana.j2
@@ -9,7 +9,7 @@ spec:
       serviceaccounts.openshift.io/oauth-redirectreference.primary: '{{ grafana_oauth_redir_ref | to_json }}'
   deployment:
     annotations:
-      hash-of-creds-to-force-restart-if-changed: {{ (servicetelemetry_vars.graphing.grafana.admin_user + servicetelemetry_vars.graphing.grafana.admin_password) | password_hash('sha256', (session_secret | b64encode)[:16] ) }}
+      hash-of-creds-to-force-restart-if-changed: {{ grafana_htpasswd_auth_string | b64encode }}
   baseImage: {{ servicetelemetry_vars.graphing.grafana.base_image }}
   ingress:
     enabled: {{ servicetelemetry_vars.graphing.grafana.ingress_enabled }}

diff --git a/roles/servicetelemetry/vars/dummy_user_certs.yml b/roles/servicetelemetry/vars/dummy_user_certs.yml
@@ -53,4 +53,4 @@ elastic_user_key_dummy: |
   njxhAQKBgQD0lOpKtL8qz9gmqtkhDRe+EPHSX8rfirqqRrPUiwK7kAJeW2vtU8aa
   hFT7lEDjb7ERyZfybIkTVVBipKx2yse9nE+1dPGIgZop3E1guDuF9aOAzIUd/+/s
   CI7s/lIBZsPD3PyxXXRtsvN7iUv5tLvNFhfomB7miTYHE+MC5QHJVQ==
-  -----END RSA PRIVATE KEY-----
+  -----END RSA PRIVATE KEY-----