Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove basic-auth method from grafana #550

Merged
merged 3 commits into from
Dec 7, 2023
Merged

Remove basic-auth method from grafana #550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99657f5
Remove basic-auth method from grafana
csibbitt Dec 5, 2023
d61030a
Remove grafana basicauth vars from defaults
csibbitt Dec 5, 2023
75c1f31
Merge branch 'master' into csibbitt/STF-1629_remove-grafana-basic-auth
csibbitt Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions deploy/crds/infra.watch_servicetelemetrys_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,13 +291,6 @@ spec:
ingressEnabled:
description: Whether to enable ingress access to Grafana
type: boolean
adminPassword:
description: Grafana admin password
type: string
format: password
adminUser:
description: Grafana admin user
type: string
baseImage:
description: Path to the base container image used to instantiate a Grafana instance
type: string
Expand Down
2 changes: 0 additions & 2 deletions deploy/crds/infra.watch_v1beta1_servicetelemetry_cr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ spec:
enabled: false
grafana:
ingressEnabled: true
adminPassword: secret
adminUser: root
disableSignoutMenu: false
baseImage: registry.redhat.io/rhel8/grafana:7
dashboards:
Expand Down
7 changes: 0 additions & 7 deletions ...y/olm-catalog/service-telemetry-operator/manifests/infra.watch_servicetelemetrys_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,13 +385,6 @@ spec:
grafana:
description: Grafana related configuration
properties:
adminPassword:
description: Grafana admin password
format: password
type: string
adminUser:
description: Grafana admin user
type: string
baseImage:
description: Path to the base container image used to instantiate
a Grafana instance
Expand Down
2 changes: 0 additions & 2 deletions ...ervice-telemetry-operator/manifests/service-telemetry-operator.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@ metadata:
"graphing": {
"enabled": false,
"grafana": {
"adminPassword": "secret",
"adminUser": "root",
"baseImage": "registry.redhat.io/rhel8/grafana:7",
"dashboards": {
"enabled": true
Expand Down
2 changes: 0 additions & 2 deletions roles/servicetelemetry/defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ servicetelemetry_defaults:
enabled: false
grafana:
ingress_enabled: true
admin_password: secret
admin_user: root
disable_signout_menu: false
base_image: registry.redhat.io/rhel8/grafana:7
dashboards:
Expand Down
48 changes: 0 additions & 48 deletions roles/servicetelemetry/tasks/component_grafana.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,54 +7,6 @@
kind: Route
name: 'grafana-route'

- 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
kind: Secret
metadata:
name: '{{ ansible_operator_meta.name }}-grafana-htpasswd'
namespace: '{{ ansible_operator_meta.namespace }}'
type: Opaque
stringData:
auth: '{{ grafana_htpasswd_auth_string }}'

- name: Lookup template
debug:
msg: "{{ lookup('template', './manifest_grafana.j2') | from_yaml }}"
Expand Down
9 changes: 1 addition & 8 deletions roles/servicetelemetry/templates/manifest_grafana.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ spec:
serviceAccount:
annotations:
serviceaccounts.openshift.io/oauth-redirectreference.primary: '{{ grafana_oauth_redir_ref | to_json }}'
deployment:
annotations:
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 }}
Expand Down Expand Up @@ -40,13 +37,12 @@ spec:
- -provider=openshift
- -pass-basic-auth=false
- -https-address=:3002
- -htpasswd-file=/etc/proxy/htpasswd/auth
- -tls-cert=/etc/tls/private/tls.crt
- -tls-key=/etc/tls/private/tls.key
- -upstream=http://localhost:3000
- -cookie-secret-file=/etc/proxy/secrets/session_secret
- -openshift-service-account=grafana-serviceaccount
- '-openshift-sar={"resource": "namespaces", "verb": "get"}'
- '-openshift-sar={"namespace":"{{ ansible_operator_meta.namespace }}","resource": "grafana", "group":"integreatly.org", "verb":"get"}'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another reminder for myself that this sort of change is release-notes worthy. Basically I'm proposing that instead of requiring accounts with cluster-wide admin-like permissions (the ability to see all namespaces) we allow any accounts with access to read grafana objects in our namespace.

Copy link
Collaborator Author

@csibbitt csibbitt Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

FWIW, making this SAR the same as the one that I'm proposing we use on prometheus (permission to read prometheus objects in our namespace) would also make sense to me.

Copy link
Member

Choose a reason for hiding this comment

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

I had to read through https://github.com/openshift/oauth-proxy#limiting-access-to-users to get a better understanding about what this was doing, but now I get it :)

- -openshift-ca=/etc/pki/tls/cert.pem
- -openshift-ca=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt
ports:
Expand All @@ -58,12 +54,9 @@ spec:
name: secret-{{ ansible_operator_meta.name }}-grafana-proxy-tls
- mountPath: /etc/proxy/secrets
name: secret-{{ ansible_operator_meta.name }}-session-secret
- mountPath: /etc/proxy/htpasswd
name: secret-{{ ansible_operator_meta.name }}-grafana-htpasswd
secrets:
- '{{ ansible_operator_meta.name }}-grafana-proxy-tls'
- '{{ ansible_operator_meta.name }}-session-secret'
- '{{ ansible_operator_meta.name }}-grafana-htpasswd'
service:
ports:
- name: web
Expand Down