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

Bugfix/fix kk configure reconciliation #589

Merged
merged 5 commits into from
Jan 17, 2025
Merged

Bugfix/fix kk configure reconciliation #589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9de782e
fix
klinch0 Jan 16, 2025
191c8b4
Merge branch 'aenix-io:main' into main
klinch0 Jan 16, 2025
7afb85e
bugfix/fix-kk-configure-reconciliation
klinch0 Jan 17, 2025
2b3c1d3
Update packages/core/platform/bundles/paas-hosted.yaml
klinch0 Jan 17, 2025
b33d348
Update packages/core/platform/bundles/paas-full.yaml
klinch0 Jan 17, 2025
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
3 changes: 3 additions & 0 deletions packages/core/platform/bundles/paas-full.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,4 +299,7 @@ releases:
chart: cozy-keycloak-configure
namespace: cozy-keycloak
dependsOn: [keycloak-operator]
values:
cozystack:
configHash: {{ $cozyConfig | toJson | sha256sum }}
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Consider using a deterministic approach for configuration hashing

The current implementation hashes the entire ConfigMap using toJson, which may lead to inconsistent hashes due to non-deterministic key ordering in JSON serialization. This could trigger unnecessary reconciliations of the keycloak-configure release.

Recommended solutions:

  • Extract and sort only the relevant configuration fields before hashing
  • Use a template function to create an ordered string representation of the required fields

Example approach:

configHash: {{ printf "%s:%s:%s" $cozyConfig.data.oidc-enabled $cozyConfig.data.root-host $cozyConfig.data.api-server-endpoint | sha256sum }}
🔗 Analysis chain

Verify the hash computation's stability across reconciliations.

The toJson conversion of the entire ConfigMap might lead to inconsistent hashes if the fields are not ordered consistently. Consider either:

  1. Sorting the keys before hashing
  2. Hashing only specific fields that affect the Keycloak configuration

Let's verify the ConfigMap structure and potential hash variations:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the ConfigMap structure and field ordering

# Get the ConfigMap structure
kubectl get configmap cozystack -n cozy-system -o yaml

# Check if there are any other consumers of this hash
rg -l "configHash" --type yaml

Length of output: 279

Script:

#!/bin/bash
# Check how $cozyConfig is constructed and used
rg -A 5 -B 5 "\$cozyConfig" --type yaml

# Look for any helper functions or templates that might handle JSON processing
fd "helpers|functions" --type yaml --exec cat {}

# Get full content of both files to compare usage
cat packages/core/platform/bundles/paas-hosted.yaml packages/core/platform/bundles/paas-full.yaml

Length of output: 13942

{{- end }}
3 changes: 3 additions & 0 deletions packages/core/platform/bundles/paas-hosted.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,7 @@ releases:
chart: cozy-keycloak-configure
namespace: cozy-keycloak
dependsOn: [keycloak-operator]
values:
cozystack:
configHash: {{ $cozyConfig | toJson | sha256sum }}
{{- end }}