Skip to content

Normalize resource key before grouping manifests to convert default namespace to empty #5774

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.

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

Merged
merged 1 commit into from
Apr 25, 2025

Conversation

ffjlabo
Copy link
Member

@ffjlabo ffjlabo commented Apr 23, 2025

What this PR does:

as title

Why we need it:

When no namespace is set for resources in the app.pipecd.yaml or actual resource manifests, there is the diffs like below.

PipeCD

When loading manifests from git, set an empty string as the namespace if no namespace is set.
But when loading manifests from actual clusters, the value default is set as the namespace.

So we need to normalize the namespace before comparing the manifests from git and the ones from the actual cluster.
Especially set empty value considering the case when no namespace is set in the manifests from git repo.

Which issue(s) this PR fixes:

Part of #5006 #5764

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

Copy link

codecov bot commented Apr 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 27.08%. Comparing base (61169c4) to head (9106631).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5774   +/-   ##
=======================================
  Coverage   27.08%   27.08%           
=======================================
  Files         504      504           
  Lines       53187    53187           
=======================================
  Hits        14405    14405           
  Misses      37708    37708           
  Partials     1074     1074           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ffjlabo ffjlabo marked this pull request as ready for review April 24, 2025 02:00
@ffjlabo ffjlabo enabled auto-merge (squash) April 24, 2025 03:57
Copy link
Contributor

@Warashi Warashi left a comment

Choose a reason for hiding this comment

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

LGTM

@ffjlabo ffjlabo merged commit acf9ba5 into master Apr 25, 2025
19 checks passed
@ffjlabo ffjlabo deleted the normalize-resource-key-before-grouping-manifests branch April 25, 2025 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants