Skip to content

Web 471/postgress pipeline#2

Closed
itaydj wants to merge 3 commits into
mainfrom
WEB-471/postgress-pipeline
Closed

Web 471/postgress pipeline#2
itaydj wants to merge 3 commits into
mainfrom
WEB-471/postgress-pipeline

Conversation

@itaydj
Copy link
Copy Markdown
Contributor

@itaydj itaydj commented Mar 2, 2026

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 2, 2026

❌ Version Check Failed

❌ Chart.yaml version (0.1.0) must be bumped from latest release (0.1.0)

Required Action

Please update the version field in Chart.yaml to be greater than the latest release.

Tip: Use semantic versioning:

  • Patch (x.y.Z): Bug fixes, small changes
  • Minor (x.Y.0): New features, backward compatible
  • Major (X.0.0): Breaking changes

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Add version check workflow and improve Helm release pipeline

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• Add PR version check workflow to enforce semantic versioning
• Improve release workflow with dependency building and index management
• Align service naming with Bitnami deployment standards
• Enhance Helm chart naming logic for release name handling
Diagram
flowchart LR
  PR["Pull Request"] -->|triggers| VersionCheck["PR Version Check<br/>Workflow"]
  VersionCheck -->|validates| ChartVersion["Chart.yaml<br/>Version Bump"]
  ChartVersion -->|posts result| PRComment["PR Comment<br/>with Status"]
  Release["Release Workflow"] -->|builds| Dependencies["Chart<br/>Dependencies"]
  Dependencies -->|packages| HelmChart["Helm Chart<br/>Package"]
  HelmChart -->|creates| GitRelease["GitHub Release"]
  GitRelease -->|updates| RepoIndex["Helm Repository<br/>Index"]
  RepoIndex -->|pushes to| GHPages["gh-pages Branch"]
Loading

Grey Divider

File Changes

1. .github/workflows/pr-version-check.yml 🧪 Tests +148/-0

New PR version validation workflow

• New workflow file that validates Chart.yaml version bumps on pull requests
• Compares PR version against latest release tag using semantic versioning
• Posts automated comments on PR with version check results
• Fails the check if version is not bumped or not greater than latest release

.github/workflows/pr-version-check.yml


2. .github/workflows/release.yml ✨ Enhancement +49/-7

Enhanced release workflow with better dependency handling

• Replace helm lint with dependency build step using Chart.lock
• Improve Helm repository index generation with conditional merge logic
• Add chart packaging output listing and cleanup of temporary directories
• Add release summary step with installation instructions for successful releases

.github/workflows/release.yml


3. templates/_helpers.tpl ✨ Enhancement +2/-0

Improve fullname template naming logic

• Add additional condition to handle cases where release name contains chart name
• Improve fullname template logic for better name generation flexibility

templates/_helpers.tpl


View more (1)
4. templates/service.yaml ⚙️ Configuration changes +1/-1

Align service naming with Bitnami standards

• Replace custom service name helper with direct release name reference
• Align service naming to use {{ .Release.Name }}-postgresql format
• Simplifies service naming to match Bitnami deployment conventions

templates/service.yaml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Mar 2, 2026

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Service name mismatch 🐞 Bug ✓ Correctness
Description
The primary Service is renamed to {{ .Release.Name }}-postgresql, but NOTES.txt (and the
existing postgresql.primary.svcName helper) still point users to a different service name derived
from postgresql.fullname. This will cause the documented DNS/psql/port-forward commands to fail
and also bypass nameOverride/fullnameOverride for the Service name.
Code

templates/service.yaml[5]

+  name: {{ .Release.Name }}-postgresql
Evidence
The primary Service is now hard-coded to {{ .Release.Name }}-postgresql, while NOTES instructs
users to connect to {{ include "postgresql.primary.svcName" . }}; that helper resolves to
postgresql.fullname, so the hostname/Service name in NOTES will not exist after this change.

templates/service.yaml[1-12]
templates/NOTES.txt[5-8]
templates/_helpers.tpl[157-159]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The primary Service name is hard-coded to `{{ .Release.Name }}-postgresql`, but the chart’s helper (`postgresql.primary.svcName`) and `NOTES.txt` still reference a different name derived from `postgresql.fullname`. This breaks the documented connection instructions and makes naming overrides inconsistent.

### Issue Context
- `templates/service.yaml` uses a hard-coded name.
- `templates/NOTES.txt` uses `postgresql.primary.svcName`.
- `postgresql.primary.svcName` resolves to `postgresql.fullname`.

### Fix Focus Areas
- templates/service.yaml[1-8]
- templates/NOTES.txt[5-8]
- templates/_helpers.tpl[150-159]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. fullname collision risk 🐞 Bug ⛯ Reliability
Description
postgresql.fullname can now return the bare chart name when the release name is a substring of the
chart name, which can cause multiple releases in the same namespace to generate identical resource
names. This can lead to install/upgrade failures and cross-release resource collisions for
StatefulSet/Secret/NetworkPolicy names that use postgresql.fullname.
Code

templates/_helpers.tpl[R16-22]

{{- $name := default .Chart.Name .Values.nameOverride }}
{{- if contains $name .Release.Name }}
{{- .Release.Name | trunc 63 | trimSuffix "-" }}
+{{- else if contains .Release.Name $name }}
+{{- $name | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
Evidence
The new branch else if contains .Release.Name $name returns $name (chart name) instead of
including the release name. Since multiple resources use include "postgresql.fullname" for
metadata.name, any two releases whose names are substrings of the chart name (e.g., postgres,
webrix) can collide on the same resource names.

templates/_helpers.tpl[12-23]
templates/statefulset.yaml[1-6]
templates/secret.yaml[1-8]
templates/networkpolicy.yaml[3-6]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`postgresql.fullname` may return the chart name without the release name when the release name is a substring of the chart name. Since many resources use `postgresql.fullname` for `metadata.name`, this can create cross-release naming collisions and break installs/upgrades.

### Issue Context
The added branch:
- triggers when `contains .Release.Name $name` (i.e., `$name` contains the release name)
- returns `$name` only

### Fix Focus Areas
- templates/_helpers.tpl[12-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Latest tag selection 🐞 Bug ⛯ Reliability
Description
The PR version check compares Chart.yaml against only the first tag returned by the GitHub tags
API (.[0].name), which is not guaranteed to be the highest semantic version or the latest release.
This can cause incorrect merge blocking or missed version-bump enforcement.
Code

.github/workflows/pr-version-check.yml[R41-55]

+          # Get latest release/tag version
+          latest_tag=$(gh api repos/${{ github.repository }}/tags --jq '.[0].name' 2>/dev/null || echo "")
+
+          if [ -z "$latest_tag" ]; then
+            echo "ℹ️  No existing releases found"
+            echo "✅ This will be the first release with version: $pr_version"
+            echo "status=success" >> $GITHUB_OUTPUT
+            echo "message=✅ First release - version $pr_version will be used" >> $GITHUB_OUTPUT
+            exit 0
+          fi
+
+          # Remove 'v' prefix and chart name prefix if present
+          # Handles both "v0.1.32" and "webrix-postgres-0.1.32" formats
+          latest_version=$(echo "$latest_tag" | sed -E 's/^v//;s/^[a-zA-Z-]+-//')
+          echo "📦 Latest release version: $latest_version"
Evidence
The workflow derives latest_tag from /tags and assumes the first entry is the one to compare
against. The rest of the script then treats that single tag as the baseline for version bump
validation.

.github/workflows/pr-version-check.yml[41-55]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The PR version check uses the first element from the GitHub `/tags` API response as the &quot;latest&quot; version, which may not represent the latest semantic version/release.

### Issue Context
This impacts merge gating and can produce false failures or false passes depending on tagging practices.

### Fix Focus Areas
- .github/workflows/pr-version-check.yml[41-55]
- .github/workflows/pr-version-check.yml[65-99]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Release lacks lint 🐞 Bug ⛯ Reliability
Description
The release workflow packages and publishes the Helm chart without running helm lint (or other
chart validation) first. This increases the likelihood of publishing a broken chart to GitHub
Releases/gh-pages.
Code

.github/workflows/release.yml[R60-72]

+      - name: Build chart dependencies
        if: steps.check_release.outputs.exists == 'false'
-        run: helm lint .
+        run: |
+          echo "🔨 Building chart dependencies..."
+          if [ -f "Chart.lock" ]; then
+            helm dependency build
+            echo "✅ Dependencies built"
+          else
+            echo "ℹ️  No dependencies to build"
+          fi

      - name: Package Helm chart
        if: steps.check_release.outputs.exists == 'false'
Evidence
In the release job, after dependency build it immediately runs helm package and creates the GitHub
release; no lint/template validation step exists in between in the shown sequence.

.github/workflows/release.yml[60-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The release workflow publishes the chart without running Helm validation. This can ship templating/YAML issues directly to users.

### Issue Context
A `helm lint` (and optionally `helm template` smoke check) step should run after dependencies are built and before packaging.

### Fix Focus Areas
- .github/workflows/release.yml[60-90]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: check_version_bump

Failed stage: Check Chart.yaml version bump [❌]

Failed test name: ""

Failure summary:

The action failed because the Helm chart Chart.yaml version was not bumped in the PR.
- Detected PR
Chart.yaml version: 0.1.0
- Detected latest release version: 0.1.0
- The version-check step requires
pr_version to be greater than latest_version, but they were equal, so it set status=failure and
exited with code 1 (Process completed with exit code 1).

Relevant error logs:
1:  ##[group]Runner Image Provisioner
2:  Hosted Compute Agent
...

186:  �[36;1m  echo "status=failure" >> $GITHUB_OUTPUT�[0m
187:  �[36;1m  echo "message=❌ Chart.yaml version ($pr_version) must be greater than latest release ($latest_version)" >> $GITHUB_OUTPUT�[0m
188:  �[36;1m  exit 1�[0m
189:  �[36;1mfi�[0m
190:  �[36;1m�[0m
191:  �[36;1mecho "✅ Version bump detected: $latest_version → $pr_version ($bump_type)"�[0m
192:  �[36;1mecho "status=success" >> $GITHUB_OUTPUT�[0m
193:  �[36;1mecho "message=✅ Version bumped: $latest_version → $pr_version ($bump_type)" >> $GITHUB_OUTPUT�[0m
194:  shell: /usr/bin/bash -e {0}
195:  env:
196:  GH_TOKEN: ***
197:  ##[endgroup]
198:  📄 PR Chart.yaml version: 0.1.0
199:  📦 Latest release version: 0.1.0
200:  ❌ Version not bumped!
201:  ##[error]Process completed with exit code 1.
202:  ##[group]Run actions/github-script@v7
203:  with:
204:  script: const status = 'failure';
205:  const message = '❌ Chart.yaml version (0.1.0) must be bumped from latest release (0.1.0)';
206:  
207:  const commentBody = status === 'success'
208:    ? `## ✅ Version Check Passed\n\n${message}\n\nThis PR can be merged.`
209:    : `## ❌ Version Check Failed\n\n${message}\n\n### Required Action\n\nPlease update the \`version\` field in \`Chart.yaml\` to be greater than the latest release.\n\n**Tip**: Use semantic versioning:\n- **Patch** (x.y.Z): Bug fixes, small changes\n- **Minor** (x.Y.0): New features, backward compatible\n- **Major** (X.0.0): Breaking changes`;
210:  

Comment thread templates/service.yaml
kind: Service
metadata:
name: {{ include "postgresql.primary.svcName" . }}
name: {{ .Release.Name }}-postgresql
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Service name mismatch 🐞 Bug ✓ Correctness

The primary Service is renamed to {{ .Release.Name }}-postgresql, but NOTES.txt (and the
existing postgresql.primary.svcName helper) still point users to a different service name derived
from postgresql.fullname. This will cause the documented DNS/psql/port-forward commands to fail
and also bypass nameOverride/fullnameOverride for the Service name.
Agent Prompt
### Issue description
The primary Service name is hard-coded to `{{ .Release.Name }}-postgresql`, but the chart’s helper (`postgresql.primary.svcName`) and `NOTES.txt` still reference a different name derived from `postgresql.fullname`. This breaks the documented connection instructions and makes naming overrides inconsistent.

### Issue Context
- `templates/service.yaml` uses a hard-coded name.
- `templates/NOTES.txt` uses `postgresql.primary.svcName`.
- `postgresql.primary.svcName` resolves to `postgresql.fullname`.

### Fix Focus Areas
- templates/service.yaml[1-8]
- templates/NOTES.txt[5-8]
- templates/_helpers.tpl[150-159]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread templates/_helpers.tpl
Comment on lines 16 to 22
{{- $name := default .Chart.Name .Values.nameOverride }}
{{- if contains $name .Release.Name }}
{{- .Release.Name | trunc 63 | trimSuffix "-" }}
{{- else if contains .Release.Name $name }}
{{- $name | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Fullname collision risk 🐞 Bug ⛯ Reliability

postgresql.fullname can now return the bare chart name when the release name is a substring of the
chart name, which can cause multiple releases in the same namespace to generate identical resource
names. This can lead to install/upgrade failures and cross-release resource collisions for
StatefulSet/Secret/NetworkPolicy names that use postgresql.fullname.
Agent Prompt
### Issue description
`postgresql.fullname` may return the chart name without the release name when the release name is a substring of the chart name. Since many resources use `postgresql.fullname` for `metadata.name`, this can create cross-release naming collisions and break installs/upgrades.

### Issue Context
The added branch:
- triggers when `contains .Release.Name $name` (i.e., `$name` contains the release name)
- returns `$name` only

### Fix Focus Areas
- templates/_helpers.tpl[12-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@itaydj itaydj closed this Mar 2, 2026
@itaydj itaydj deleted the WEB-471/postgress-pipeline branch March 2, 2026 06:45
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