Skip to content

Add rolling-updates feature flag and compatibility framework#7

Open
zaibkhan wants to merge 1 commit into
feature-rolling-updates-baselinefrom
feature-rolling-updates-implementation
Open

Add rolling-updates feature flag and compatibility framework#7
zaibkhan wants to merge 1 commit into
feature-rolling-updates-baselinefrom
feature-rolling-updates-implementation

Conversation

@zaibkhan

Copy link
Copy Markdown

No description provided.

Closes #36840

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
@codoki-pr-intelligence

codoki-pr-intelligence Bot commented Sep 10, 2025

Copy link
Copy Markdown

Codoki PR Review

Summary: Parameterize feature flag, prevent build breaks
What’s good: Concise update to enable rolling-updates in the custom Keycloak image build.
Review Status: ❌ Requires changes
Overall Priority: High
Review Update:
• Coverage: Reviewed all 11 files across 2 batches

Issues (Critical & High only)

Severity Issue Why it matters
High Compatibility — Hard-coded rolling-updates feature breaks older versions …/scripts/Dockerfile-custom-image
Hardcoding the rolling-updates feature in a generic Dockerfile can break builds when ARG VERSION points to a Keycloak release that doesn't support this feature (kc.sh will fail with an unknown feature). It also removes flexibility for consumers who may not want this feature on by default. Make the features flag opt-in via an environment variable to preserve backward compatibility while still allowing easy enablement.

Showing top 1 issues. Critical: 0, High: 1. See inline suggestions for more.

Key Feedback (click to expand)
  • Needs improvement: Hardcoding a feature flag in a generic Dockerfile risks build failures with versions that don't recognize it and removes flexibility for consumers.
  • Testing: Consider adding a lightweight CI check that builds this Dockerfile with both a version that supports rolling-updates and one that predates it (e.g., previous minor/patch), ensuring unknown features fail fast in automation rather than at user build time.
  • Documentation: If you keep this Dockerfile generic, document how to enable rolling-updates via an environment variable (e.g., KC_FEATURES=rolling-updates) when building the image, and mention compatibility expectations by Keycloak version.
  • Compatibility: Hardcoding --features=rolling-updates couples the Dockerfile to specific server versions; for older tags, kc.sh may fail with 'unknown feature'. Making the flag opt-in via an env/ARG preserves backward compatibility and allows consumers to choose features by version.
  • Open questions: Should the custom image Dockerfile be opinionated by default, or should enabling rolling-updates be opt-in via env/ARG to support mixed version use-cases?

Confidence: 3/5 — Needs work before merge (1 high · status: Requires changes)

React with 👍 or 👎 if you found this review useful.

FROM $IMAGE:$VERSION

RUN /opt/keycloak/bin/kc.sh build --db=postgres --health-enabled=true
RUN /opt/keycloak/bin/kc.sh build --db=postgres --health-enabled=true --features=rolling-updates

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ High: Hardcoding the rolling-updates feature in a generic Dockerfile can break builds when ARG VERSION points to a Keycloak release that doesn't support this feature (kc.sh will fail with an unknown feature). It also removes flexibility for consumers who may not want this feature on by default. Make the features flag opt-in via an environment variable to preserve backward compatibility while still allowing easy enablement.

Suggested change
RUN /opt/keycloak/bin/kc.sh build --db=postgres --health-enabled=true --features=rolling-updates
RUN /opt/keycloak/bin/kc.sh build --db=postgres --health-enabled=true ${KC_FEATURES:+--features=${KC_FEATURES}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants