Skip to content

[chore] Updating pullPolicy to match uptsream chart#282

Merged
openshift-merge-bot[bot] merged 3 commits intoredhat-developer:mainfrom
OpinionatedHeron:imagePullPolicy
Dec 8, 2025
Merged

[chore] Updating pullPolicy to match uptsream chart#282
openshift-merge-bot[bot] merged 3 commits intoredhat-developer:mainfrom
OpinionatedHeron:imagePullPolicy

Conversation

@OpinionatedHeron
Copy link
Copy Markdown
Member

Description of the change

Updating the pullPolicy to match the default configuration to the upstream Backstage chart.

Which issue(s) does this PR fix or relate to

RHDHBUGS-2236

Checklist

  • For each Chart updated, version bumped in the corresponding Chart.yaml according to Semantic Versioning.
  • For each Chart updated, variables are documented in the values.yaml and added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Use pre-commit run -a to apply changes. The pre-commit Workflow will do this automatically for you if needed.
  • JSON Schema template updated and re-generated the raw schema via the pre-commit hook.
  • Tests pass using the Chart Testing tool and the ct lint command.
  • If you updated the orchestrator-infra chart, make sure the versions of the Knative CRDs are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the values.yaml file. See Installing Knative Eventing and Knative Serving CRDs for more details.

Signed-off-by: Leanne Ahern <lahern@redhat.com>
@rhdh-qodo-merge
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

RHDHBUGS-2236 - Partially compliant

Compliant requirements:

  • Unset image pullPolicy values in the RHDH chart to honor Kubernetes default image pull behavior.
  • Align RHDH chart defaults with the upstream Backstage chart where pullPolicy is not set.
  • Ensure values.yaml, schema templates, and any templates/tests reflect the unset policy.

Non-compliant requirements:

  • Bump chart version and update schema/README as required by repo processes.

Requires further human verification:

  • Confirm chart version bump and README regeneration occurred outside the shown diff.
  • Run chart-testing (ct lint) to ensure schema and values remain valid with empty pullPolicy.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Behavior Change

Setting imagePullPolicy to an empty string relies on Kubernetes defaulting; verify that test pods still pull images as expected across cluster versions and runtimes.

imagePullPolicy: ""
command: ["/bin/sh", "-c"]
Schema/Defaults Consistency

Empty string for pullPolicy must be accepted by the upstream schema and templating; validate helm template output does not render an explicit empty key in places where upstream chart expects the field to be omitted.

  pullPolicy: ""
command: []
📚 Focus areas based on broader codebase context

Schema Mismatch

The schema default sets upstream.backstage.image.pullPolicy to an empty string. In the existing schema patterns, image-related fields define explicit defaults and valid values. Using an empty string may violate the upstream chart schema expectations and break validation or defaulting. Confirm the allowed enum/format for pullPolicy and set an explicit value if required. (Ref 1, Ref 4)

"backstage": {
    "image": {
        "registry": "quay.io",
        "repository": "janus-idp/redhat-backstage-build",
        "tag": "latest",
        "pullPolicy": ""
    }

Reference reasoning: The repository’s JSON schemas consistently provide explicit defaults for image and operator settings (e.g., tag defaults, booleans, and strings) rather than empty strings. Aligning with these patterns ensures compatibility with the referenced upstream schema and avoids validation issues.

📄 References
  1. redhat-developer/rhdh-chart/charts/backstage/values.schema.tmpl.json [199-305]
  2. redhat-developer/rhdh-chart/charts/orchestrator-infra/values.schema.json [1-133]
  3. redhat-developer/rhdh-chart/charts/orchestrator-infra/values.schema.tmpl.json [1-136]
  4. redhat-developer/rhdh-chart/charts/backstage/values.schema.tmpl.json [306-418]
  5. redhat-developer/rhdh-chart/charts/backstage/values.schema.tmpl.json [109-199]
  6. redhat-developer/rhdh-chart/charts/backstage/values.schema.tmpl.json [419-423]

@rhdh-qodo-merge rhdh-qodo-merge Bot added the enhancement New feature or request label Dec 8, 2025
@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Dec 8, 2025

PR Type

(Describe updated until commit 320c421)

Enhancement


Description

  • Update imagePullPolicy to empty string matching upstream Backstage chart defaults

  • Bump chart version from 4.8.0 to 4.8.1 (patch release)

  • Update documentation and schema to reflect new imagePullPolicy configuration


File Walkthrough

Relevant files
Configuration changes
Chart.yaml
Bump chart version to 4.8.1                                                           

charts/backstage/Chart.yaml

  • Increment chart version from 4.8.0 to 4.8.1 for patch release
+1/-1     
values.schema.json
Update schema defaults for imagePullPolicy                             

charts/backstage/values.schema.json

  • Update backstage image pullPolicy default from "IfNotPresent" to empty
    string
  • Update install-dynamic-plugins init container imagePullPolicy default
    from "IfNotPresent" to empty string
+2/-2     
values.schema.tmpl.json
Update template schema pullPolicy default                               

charts/backstage/values.schema.tmpl.json

  • Update upstream backstage image pullPolicy default from "IfNotPresent"
    to empty string
+1/-1     
values.yaml
Update values imagePullPolicy defaults                                     

charts/backstage/values.yaml

  • Change backstage image pullPolicy from "IfNotPresent" to empty string
  • Change install-dynamic-plugins init container imagePullPolicy from
    "IfNotPresent" to empty string
+2/-2     
Documentation
README.md
Update documentation version references                                   

charts/backstage/README.md

  • Update version badge from 4.8.0 to 4.8.1
  • Update helm install command example to use version 4.8.1
+2/-2     
Enhancement
test-connection.yaml
Update test container imagePullPolicy                                       

charts/backstage/templates/tests/test-connection.yaml

  • Change test container imagePullPolicy from "IfNotPresent" to empty
    string
+1/-1     

Signed-off-by: Leanne Ahern <lahern@redhat.com>
@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Dec 8, 2025

PR Code Suggestions ✨

Latest suggestions up to 320c421

CategorySuggestion                                                                                                                                    Impact
Possible issue
Remove invalid default in schema

In values.schema.json, remove the invalid default: "" for pullPolicy to prevent
generating invalid Kubernetes manifests.

charts/backstage/values.schema.json [4558-4566]

 "pullPolicy": {
-    "default": "",
     "description": "Ref: https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy",
     "enum": [
         "Always",
         "IfNotPresent",
         "Never"
     ],
     "title": "Image pull policy",
     "type": "string"
 },
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that setting pullPolicy's default to an empty string is invalid for the Kubernetes API, which would cause deployment failures. The proposed fix to remove the default is appropriate.

High
Remove invalid default pullPolicy

In values.schema.tmpl.json, remove the invalid pullPolicy: "" from the default
values for the upstream image to prevent generating invalid Kubernetes
manifests.

charts/backstage/values.schema.tmpl.json [7-20]

 "upstream": {
     "title": "Upstream Backstage chart schema.",
     "$ref": "https://raw.githubusercontent.com/backstage/charts/backstage-{{ dependencies | selectattr('name', 'equalto', 'backstage') | map(attribute='version') | list | join('') }}/charts/backstage/values.schema.json",
     "default": {
         "backstage": {
             "image": {
                 "registry": "quay.io",
                 "repository": "janus-idp/redhat-backstage-build",
-                "tag": "latest",
-                "pullPolicy": ""
+                "tag": "latest"
             }
         }
     }
 },

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly points out that setting pullPolicy to an empty string in the default values is invalid and will break deployments. Removing the key is a valid and robust solution.

High
Omit invalid imagePullPolicy field

In values.schema.json, remove the invalid imagePullPolicy: "" for the
install-dynamic-plugins init container to prevent generating an invalid Pod
specification.

charts/backstage/values.schema.json [4625]

-"imagePullPolicy": "",
+/* remove the line entirely, allowing omission when unspecified */
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that an empty string for imagePullPolicy is invalid and will cause deployment failures. The proposed fix to remove the line is a valid approach to resolve the issue.

High
  • More

Previous suggestions

Suggestions up to commit bd092b3
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix invalid imagePullPolicy value

The imagePullPolicy is set to an invalid empty string, which will cause pod
failure. Make this value configurable and conditionally render the key in the
template to fix the issue.

charts/backstage/templates/tests/test-connection.yaml [33-35]

 image: "{{ .Values.test.image.registry }}/{{ .Values.test.image.repository }}:{{ .Values.test.image.tag }}"
-imagePullPolicy: ""
+{{- with .Values.test.image.pullPolicy }}
+imagePullPolicy: {{ . }}
+{{- end }}
 command: ["/bin/sh", "-c"]
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that setting imagePullPolicy to an empty string is invalid and will cause pod creation to fail, proposing a robust fix that prevents a runtime error.

High
Correct invalid imagePullPolicy value

The imagePullPolicy for the install-dynamic-plugins init container is an invalid
empty string. Change it to a valid value like IfNotPresent to prevent pod
creation failure.

charts/backstage/values.yaml [216]

 ...
        - name: MAX_ENTRY_SIZE
          value: "30000000"
-    imagePullPolicy: ""
+    imagePullPolicy: IfNotPresent
     volumeMounts:
        - mountPath: /dynamic-plugins-root
          name: dynamic-plugins-root
 ...
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that an empty imagePullPolicy in values.yaml is an invalid value that will cause the install-dynamic-plugins init container to fail, preventing a critical runtime error.

High
Avoid invalid image pull policy

The pullPolicy is set to an invalid empty string, which will likely cause
deployment failure. Change it to a valid value such as IfNotPresent to ensure a
successful deployment.

charts/backstage/values.yaml [42]

 ...
    repository: rhdh-community/rhdh
    tag: next
-  pullPolicy: ""
+  pullPolicy: IfNotPresent
  command: []
  # FIXME (tumido): USE POSTGRES_PASSWORD and POSTGRES_USER instead of POSTGRES_ADMIN_PASSWORD
 ...
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that setting pullPolicy to an empty string is invalid and will likely cause the main application deployment to fail, thus preventing a critical runtime error.

High

Co-authored-by: OpinionatedHeron <OpinionatedHeron@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 8, 2025

⚠️ Files changed after running the pre-commit hooks

Those changes should have been pushed automatically to your PR branch.

NOTE: If the PR checks are stuck after this additional commit, manually close the PR and immediately reopen it to trigger the checks again.

@rm3l rm3l closed this Dec 8, 2025
@rm3l rm3l reopened this Dec 8, 2025
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Dec 8, 2025

@rhdh-qodo-merge
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

RHDHBUGS-2236 - PR Code Verified

Compliant requirements:

  • Unset image pullPolicy values in the RHDH chart so Kubernetes defaulting behavior is honored unless explicitly set by the user.
  • Align defaults with upstream Backstage chart where pullPolicy is unset.
  • Bump chart version and update README badges/instructions accordingly.
  • Regenerate/update JSON schema and schema template to reflect unset pullPolicy defaults.
  • Ensure any initContainers or test containers also honor unset pullPolicy.

Requires further human verification:

  • Validate via rendering (helm template) and a test deployment that Kubernetes applies default ImagePullPolicy as expected with various image tags (with/without tag and with digest).
  • Confirm no other charts/subcharts within the repo still set pullPolicy explicitly.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Schema Default

Setting the schema default for image.pullPolicy to an empty string should be verified against any tooling that validates enums; ensure empty string is allowed at values-level even though enum lists concrete values.

"default": "",
"description": "Ref: https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy",
"enum": [
    "Always",
    "IfNotPresent",
Test Pod Policy

Unsetting imagePullPolicy in the test Pod may affect CI flakiness depending on image tagging; verify the test environment expects Kubernetes defaults and images are tagged appropriately.

imagePullPolicy: ""
command: ["/bin/sh", "-c"]
📚 Focus areas based on broader codebase context

Schema Drift

The PR changes default image.pullPolicy values to empty strings in values and schema, which may not be validated or documented consistently. Verify that the upstream schema and our chart docs expect an empty default rather than an explicit policy, and ensure README/docs reflect this behavior. (Ref 1, Ref 2)

upstream:
  nameOverride: developer-hub
  backstage:
    image:
      registry: quay.io
      repository: rhdh-community/rhdh
      tag: next
      pullPolicy: ""
    command: []
    # FIXME (tumido): USE POSTGRES_PASSWORD and POSTGRES_USER instead of POSTGRES_ADMIN_PASSWORD
    # This is a hack. In {fedora,rhel}/postgresql images, regular user is forbidden
    # from creating DBs in runtime. A single DB can be created ahead of time via

Reference reasoning: The existing values.yaml emphasizes OpenShift-compatible defaults and explicitly configures operational settings; however, the retrieved snippets don’t show use of empty defaults for image pull policy. Aligning with upstream should include corresponding schema/defaults and documentation to prevent invalid or confusing chart configuration.

📄 References
  1. redhat-developer/rhdh-chart/charts/backstage/values.yaml [22-34]
  2. redhat-developer/rhdh-chart/charts/backstage/values.yaml [71-98]
  3. redhat-developer/rhdh-chart/charts/backstage/values.yaml [99-112]
  4. redhat-developer/rhdh-chart/charts/backstage/ci/with-orchestrator-values.yaml [1-21]
  5. redhat-developer/rhdh-chart/charts/backstage/ci/with-orchestrator-and-dynamic-plugins-npmrc-values.yaml [1-24]
  6. redhat-developer/rhdh-chart/charts/backstage/ci/with-custom-dynamic-pvc-claim-spec-values.yaml [0-2]
  7. redhat-developer/rhdh-chart/charts/orchestrator-software-templates/ci/upstream-values.yaml [1-16]
  8. redhat-developer/rhdh-chart/charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml [117-151]

@rhdh-qodo-merge
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@rhdh-qodo-merge
Copy link
Copy Markdown

Persistent suggestions updated to latest commit 320c421

Copy link
Copy Markdown
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Dec 8, 2025
@openshift-merge-bot openshift-merge-bot Bot merged commit 1597434 into redhat-developer:main Dec 8, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants