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

Refactor Keycloak #502

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Refactor Keycloak #502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 23 additions & 5 deletions hack/e2e.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ machine:
op: create

cluster:
apiServer:
extraArgs:
oidc-issuer-url: "https://keycloak.example.org/realms/cozy"
oidc-client-id: "kubernetes"
oidc-username-claim: "preferred_username"
oidc-groups-claim: "groups"
Comment on lines +127 to +132
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Verify YAML Indentation for apiServer Configuration

The indentation of the apiServer block in the patch.yaml file may be incorrect, which can lead to YAML parsing errors. Ensure that the apiServer key is properly indented under the cluster key.

Apply this diff to correct the indentation:

 cluster:
+  apiServer:
+    extraArgs:
+      oidc-issuer-url: "https://keycloak.example.org/realms/cozy"
+      oidc-client-id: "kubernetes"
+      oidc-username-claim: "preferred_username"
+      oidc-groups-claim: "groups"

Committable suggestion skipped: line range outside the PR's diff.

network:
cni:
name: none
Expand Down Expand Up @@ -182,7 +188,8 @@ timeout 60 sh -c 'until nc -nzv 192.168.123.11 50000 && nc -nzv 192.168.123.12 5
timeout 10 sh -c 'until talosctl bootstrap -n 192.168.123.11 -e 192.168.123.11; do sleep 1; done'

# Wait for etcd
timeout 180 sh -c 'while talosctl etcd members -n 192.168.123.11,192.168.123.12,192.168.123.13 -e 192.168.123.10 2>&1 | grep "rpc error"; do sleep 1; done'
timeout 180 sh -c 'until timeout -s 9 2 talosctl etcd members -n 192.168.123.11,192.168.123.12,192.168.123.13 -e 192.168.123.10 2>&1; do sleep 1; done'
timeout 60 sh -c 'while talosctl etcd members -n 192.168.123.11,192.168.123.12,192.168.123.13 -e 192.168.123.10 2>&1 | grep "rpc error"; do sleep 1; done'

rm -f kubeconfig
talosctl kubeconfig kubeconfig -e 192.168.123.10 -n 192.168.123.10
Expand All @@ -203,6 +210,8 @@ data:
ipv4-pod-gateway: "10.244.0.1"
ipv4-svc-cidr: "10.96.0.0/16"
ipv4-join-cidr: "100.64.0.0/16"
root-host: example.org
api-server-endpoint: https://192.168.123.10:6443
EOT

#
Expand Down Expand Up @@ -287,23 +296,23 @@ spec:
avoidBuggyIPs: false
EOT

kubectl patch -n tenant-root hr/tenant-root --type=merge -p '{"spec":{ "values":{
kubectl patch -n tenant-root tenants.apps.cozystack.io root --type=merge -p '{"spec":{
"host": "example.org",
"ingress": true,
"monitoring": true,
"etcd": true,
"isolated": true
}}}'
}}'

# Wait for HelmRelease be created
timeout 60 sh -c 'until kubectl get hr -n tenant-root etcd ingress monitoring tenant-root; do sleep 1; done'

# Wait for HelmReleases be installed
kubectl wait --timeout=2m --for=condition=ready -n tenant-root hr etcd ingress monitoring tenant-root

kubectl patch -n tenant-root hr/ingress --type=merge -p '{"spec":{ "values":{
kubectl patch -n tenant-root ingresses.apps.cozystack.io ingress --type=merge -p '{"spec":{
"dashboard": true
}}}'
}}'

# Wait for nginx-ingress-controller
timeout 60 sh -c 'until kubectl get deploy -n tenant-root root-ingress-controller; do sleep 1; done'
Expand All @@ -326,3 +335,12 @@ ip=$(kubectl get svc -n tenant-root root-ingress-controller -o jsonpath='{.statu

# Check Grafana
curl -sS -k "https://$ip" -H 'Host: grafana.example.org' | grep Found


# Test OIDC
kubectl patch -n cozy-system cm/cozystack --type=merge -p '{"data":{
"oidc-enabled": "true"
}}'

timeout 60 sh -c 'until kubectl get hr -n cozy-keycloak keycloak keycloak-configure keycloak-operator; do sleep 1; done'
kubectl wait --timeout=10m --for=condition=ready -n cozy-keycloak hr keycloak keycloak-configure keycloak-operator
13 changes: 13 additions & 0 deletions packages/apps/tenant/templates/dashboard-resourcemap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ include "tenant.name" . }}-dashboard-resources
namespace: {{ .Release.namespace }}
rules:
- apiGroups:
- ""
resources:
- secrets
resourceNames:
- kubeconfig-{{ include "tenant.name" . }}
verbs: ["get", "list", "watch"]
Comment on lines +1 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix YAML Syntax Error in Role Definition

A YAML syntax error is reported at line 4: "expected <block end>, but found <scalar>". This suggests there may be an issue with the indentation or structure of the metadata fields.

Apply this diff to fix the syntax error:

 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
-  name: {{ include "tenant.name" . }}-dashboard-resources
+  name: "{{ include "tenant.name" . }}-dashboard-resources"
   namespace: {{ .Release.namespace }}
 rules:

Wrapping the name value with quotes can resolve issues when template functions are used.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ include "tenant.name" . }}-dashboard-resources
namespace: {{ .Release.namespace }}
rules:
- apiGroups:
- ""
resources:
- secrets
resourceNames:
- kubeconfig-{{ include "tenant.name" . }}
verbs: ["get", "list", "watch"]
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: "{{ include "tenant.name" . }}-dashboard-resources"
namespace: {{ .Release.namespace }}
rules:
- apiGroups:
- ""
resources:
- secrets
resourceNames:
- kubeconfig-{{ include "tenant.name" . }}
verbs: ["get", "list", "watch"]
🧰 Tools
🪛 yamllint (1.35.1)

[error] 4-4: syntax error: expected , but found ''

(syntax)

4 changes: 4 additions & 0 deletions packages/apps/tenant/templates/keycloakgroups.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
{{- $cozyConfig := lookup "v1" "ConfigMap" "cozy-system" "cozystack" }}
{{- $oidcEnabled := index $cozyConfig.data "oidc-enabled" }}
{{- if $oidcEnabled }}
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for missing ConfigMap

The ConfigMap lookup could fail if cozystack ConfigMap doesn't exist in the cozy-system namespace. Consider adding a fallback or error handling mechanism.

 {{- $cozyConfig := lookup "v1" "ConfigMap" "cozy-system" "cozystack" }}
+{{- if not $cozyConfig }}
+  {{- fail "Required ConfigMap 'cozystack' not found in 'cozy-system' namespace" }}
+{{- end }}
 {{- $oidcEnabled := index $cozyConfig.data "oidc-enabled" }}
 {{- if $oidcEnabled }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{{- $cozyConfig := lookup "v1" "ConfigMap" "cozy-system" "cozystack" }}
{{- $oidcEnabled := index $cozyConfig.data "oidc-enabled" }}
{{- if $oidcEnabled }}
{{- $cozyConfig := lookup "v1" "ConfigMap" "cozy-system" "cozystack" }}
{{- if not $cozyConfig }}
{{- fail "Required ConfigMap 'cozystack' not found in 'cozy-system' namespace" }}
{{- end }}
{{- $oidcEnabled := index $cozyConfig.data "oidc-enabled" }}
{{- if $oidcEnabled }}
🧰 Tools
🪛 yamllint (1.35.1)

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

apiVersion: v1.edp.epam.com/v1
kind: KeycloakRealmGroup
metadata:
Expand Down Expand Up @@ -47,3 +50,4 @@ spec:
realmRef:
name: keycloakrealm-cozy
kind: ClusterKeycloakRealm
{{- end }}
24 changes: 5 additions & 19 deletions packages/apps/tenant/templates/kubeconfig.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,13 @@
{{- $cozyConfig := lookup "v1" "ConfigMap" "cozy-system" "cozystack" }}
{{- $host := index $cozyConfig.data "root-host" }}
{{- $apiServerAdress := index $cozyConfig.data "api-server-adress" }}
{{- $k8sClientSecret := lookup "v1" "Secret" "cozy-keycloak" "k8s-client" }}

{{- if $k8sClientSecret }}
{{- $apiServerEndpoint := index $cozyConfig.data "api-server-endpoint" }}
{{- $k8sClient := index $k8sClientSecret.data "client-secret-key" | b64dec }}
{{- $rootSaConfigMap := lookup "v1" "ConfigMap" "kube-system" "kube-root-ca.crt" }}
Comment on lines +4 to 8
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for required configuration values

The template assumes all required values exist in ConfigMaps/Secrets. Add validation to ensure required values are present and valid.

 {{- if $k8sClientSecret }}
+{{- if not (index $cozyConfig.data "api-server-endpoint") }}
+  {{- fail "Required value 'api-server-endpoint' not found in cozystack ConfigMap" }}
+{{- end }}
 {{- $apiServerEndpoint := index $cozyConfig.data "api-server-endpoint" }}
 {{- $k8sClient := index $k8sClientSecret.data "client-secret-key" | b64dec }}
+{{- if not $k8sClient }}
+  {{- fail "Empty client secret found in k8s-client Secret" }}
+{{- end }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{{- if $k8sClientSecret }}
{{- $apiServerEndpoint := index $cozyConfig.data "api-server-endpoint" }}
{{- $k8sClient := index $k8sClientSecret.data "client-secret-key" | b64dec }}
{{- $rootSaConfigMap := lookup "v1" "ConfigMap" "kube-system" "kube-root-ca.crt" }}
{{- if $k8sClientSecret }}
{{- if not (index $cozyConfig.data "api-server-endpoint") }}
{{- fail "Required value 'api-server-endpoint' not found in cozystack ConfigMap" }}
{{- end }}
{{- $apiServerEndpoint := index $cozyConfig.data "api-server-endpoint" }}
{{- $k8sClient := index $k8sClientSecret.data "client-secret-key" | b64dec }}
{{- if not $k8sClient }}
{{- fail "Empty client secret found in k8s-client Secret" }}
{{- end }}
{{- $rootSaConfigMap := lookup "v1" "ConfigMap" "kube-system" "kube-root-ca.crt" }}

{{- $k8sCa := index $rootSaConfigMap.data "ca.crt" | b64enc }}

apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ include "tenant.name" . }}-dashboard-resources
namespace: {{ .Release.namespace }}
rules:
- apiGroups:
- ""
resources:
- secrets
resourceNames:
- kubeconfig-{{ include "tenant.name" . }}
verbs: ["get", "list", "watch"]


---

apiVersion: v1
kind: Secret
metadata:
Expand All @@ -33,7 +18,7 @@ stringData:
apiVersion: v1
clusters:
- cluster:
server: https://{{ $apiServerAdress }}:6443
server: {{ $apiServerEndpoint }}
certificate-authority-data: {{ $k8sCa }}
name: cluster
contexts:
Expand All @@ -57,3 +42,4 @@ stringData:
- --skip-open-browser
- --grant-type=password
command: kubectl
{{- end }}
7 changes: 0 additions & 7 deletions packages/core/platform/bundles/distro-full.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,3 @@ releases:
namespace: cozy-keycloak
optional: true
dependsOn: [keycloak]

- name: keycloak-configure
releaseName: keycloak-configure
chart: cozy-keycloak-configure
namespace: cozy-keycloak
optional: true
dependsOn: [keycloak-operator]
7 changes: 0 additions & 7 deletions packages/core/platform/bundles/distro-hosted.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,3 @@ releases:
namespace: cozy-keycloak
optional: true
dependsOn: [keycloak]

- name: keycloak-configure
releaseName: keycloak-configure
chart: cozy-keycloak-configure
namespace: cozy-keycloak
optional: true
dependsOn: [keycloak-operator]
20 changes: 16 additions & 4 deletions packages/core/platform/bundles/paas-full.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
{{- $cozyConfig := lookup "v1" "ConfigMap" "cozy-system" "cozystack" }}
{{- $oidcEnabled := index $cozyConfig.data "oidc-enabled" }}
{{- $host := index $cozyConfig.data "root-host" }}
{{- if not $host }}
{{- fail "ERROR need root-host in cozystack ConfigMap" }}
{{- end }}
{{- $apiServerEndpoint := index $cozyConfig.data "api-server-endpoint" }}
{{- if not $apiServerEndpoint }}
{{- fail "ERROR need api-server-endpoint in cozystack ConfigMap" }}
{{- end }}

releases:
- name: fluxcd-operator
Expand Down Expand Up @@ -205,10 +210,6 @@ releases:
chart: cozy-dashboard
namespace: cozy-dashboard
dependsOn: [cilium,kubeovn,keycloak-configure]
valuesFrom:
- kind: ConfigMap
name: kubeapps-auth-config
valuesKey: values.yaml
{{- if .Capabilities.APIVersions.Has "source.toolkit.fluxcd.io/v1" }}
{{- with (lookup "source.toolkit.fluxcd.io/v1" "HelmRepository" "cozy-public" "").items }}
values:
Expand All @@ -222,6 +223,15 @@ releases:
{{- end }}
{{- end }}
{{- end }}
{{- if $oidcEnabled }}
dependsOn: [keycloak-configure]
valuesFrom:
- kind: ConfigMap
name: kubeapps-auth-config
valuesKey: values.yaml
{{- else }}
dependsOn: []
{{- end }}
Comment on lines +226 to +234
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix conflicting dependsOn declarations

There's a conflict between the initial dependsOn list [cilium,kubeovn,keycloak-configure] and the conditional block that overwrites it. This could lead to losing the cilium and kubeovn dependencies.

Suggested fix:

  {{- if $oidcEnabled }}
- dependsOn: [keycloak-configure]
+ dependsOn: [cilium,kubeovn,keycloak-configure]
  valuesFrom:
    - kind: ConfigMap
      name: kubeapps-auth-config
      valuesKey: values.yaml
  {{- else }}
- dependsOn: []
+ dependsOn: [cilium,kubeovn]
  {{- end }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{{- if $oidcEnabled }}
dependsOn: [keycloak-configure]
valuesFrom:
- kind: ConfigMap
name: kubeapps-auth-config
valuesKey: values.yaml
{{- else }}
dependsOn: []
{{- end }}
{{- if $oidcEnabled }}
dependsOn: [cilium,kubeovn,keycloak-configure]
valuesFrom:
- kind: ConfigMap
name: kubeapps-auth-config
valuesKey: values.yaml
{{- else }}
dependsOn: [cilium,kubeovn]
{{- end }}


- name: kamaji
releaseName: kamaji
Expand Down Expand Up @@ -257,6 +267,7 @@ releases:
optional: true
dependsOn: [cilium,kubeovn]

{{- if $oidcEnabled }}
- name: keycloak
releaseName: keycloak
chart: cozy-keycloak
Expand All @@ -274,3 +285,4 @@ releases:
chart: cozy-keycloak-configure
namespace: cozy-keycloak
dependsOn: [keycloak-operator]
{{- end }}
21 changes: 16 additions & 5 deletions packages/core/platform/bundles/paas-hosted.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
{{- $cozyConfig := lookup "v1" "ConfigMap" "cozy-system" "cozystack" }}
{{- $oidcEnabled := index $cozyConfig.data "oidc-enabled" }}
{{- $host := index $cozyConfig.data "root-host" }}
{{- if not $host }}
{{- fail "ERROR need root-host in cozystack ConfigMap" }}
{{- end }}
{{- $apiServerEndpoint := index $cozyConfig.data "api-server-endpoint" }}
{{- if not $apiServerEndpoint }}
{{- fail "ERROR need api-server-endpoint in cozystack ConfigMap" }}
{{- end }}

releases:
- name: fluxcd-operator
Expand Down Expand Up @@ -134,11 +139,6 @@ releases:
releaseName: dashboard
chart: cozy-dashboard
namespace: cozy-dashboard
dependsOn: [keycloak-configure]
valuesFrom:
- kind: ConfigMap
name: kubeapps-auth-config
valuesKey: values.yaml
{{- if .Capabilities.APIVersions.Has "source.toolkit.fluxcd.io/v1" }}
{{- with (lookup "source.toolkit.fluxcd.io/v1" "HelmRepository" "cozy-public" "").items }}
values:
Expand All @@ -153,7 +153,17 @@ releases:
{{- end }}
{{- end }}
{{- end }}
{{- if $oidcEnabled }}
dependsOn: [keycloak-configure]
valuesFrom:
- kind: ConfigMap
name: kubeapps-auth-config
valuesKey: values.yaml
{{- else }}
dependsOn: []
{{- end }}

{{- if $oidcEnabled }}
- name: keycloak
releaseName: keycloak
chart: cozy-keycloak
Expand All @@ -171,3 +181,4 @@ releases:
chart: cozy-keycloak-configure
namespace: cozy-keycloak
dependsOn: [keycloak-operator]
{{- end }}
69 changes: 1 addition & 68 deletions packages/system/keycloak-configure/templates/configure-kk.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{{- $cozyConfig := lookup "v1" "ConfigMap" "cozy-system" "cozystack" }}
{{- $host := index $cozyConfig.data "root-host" }}
{{- $apiServerAdress := index $cozyConfig.data "api-server-adress" }}
{{- $k8sClient := randAlphaNum 32 -}}
{{- $kubeappsClient := randAlphaNum 32 -}}
{{- $rootSaConfigMap := lookup "v1" "ConfigMap" "kube-system" "kube-root-ca.crt" }}
Expand Down Expand Up @@ -82,7 +81,7 @@ spec:
clientId: kubernetes
directAccess: true
public: false
webUrl: https://{{ $apiServerAdress }}/oauth2/callback
webUrl: https://localhost:8000/oauth2/callback
webOrigins:
- /*
defaultClientScopes:
Expand Down Expand Up @@ -175,69 +174,3 @@ data:
- --cookie-secure=false
- --scope=openid email groups
- --oidc-issuer-url=https://keycloak.{{ $host }}/realms/cozy

---

apiVersion: v1.edp.epam.com/v1
kind: KeycloakRealmGroup
metadata:
name: kubeapps-admin
namespace: {{ include "tenant.name" . }}
spec:
name: kubeapps-admin
realmRef:
name: keycloakrealm-cozy
kind: ClusterKeycloakRealm

---

apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: tenant-root-dashboard-resources
namespace: tenant-root
rules:
- apiGroups:
- ""
resources:
- secrets
resourceNames:
- kubeconfig
verbs: ["get", "list", "watch"]


---

apiVersion: v1
kind: Secret
metadata:
name: kubeconfig
namespace: tenant-root
stringData:
kubeconfig: |
apiVersion: v1
clusters:
- cluster:
server: https://{{ $apiServerAdress }}:6443
certificate-authority-data: {{ $k8sCa }}
name: cluster
contexts:
- context:
cluster: cluster
user: keycloak
name: default
current-context: default
users:
- name: keycloak
user:
exec:
apiVersion: client.authentication.k8s.io/v1beta1
args:
- oidc-login
- get-token
- --oidc-issuer-url=https://keycloak.{{ $host }}/realms/cozy
- --oidc-client-id=kubernetes
- --oidc-client-secret={{ $k8sClient }}
- --skip-open-browser
- --grant-type=password
command: kubectl
Loading