Repository navigation
fix: clean configured chart dirs before matching chart directories - #878
Open
RaphaelFakhri wants to merge 1 commit into
Open
RaphaelFakhri wants to merge 1 commit into
RaphaelFakhri wants to merge 1 commit into
Conversation
LookupChartDir compares the cleaned parent of a candidate chart directory with the configured chart directory, but only stripped trailing slashes from the configured value. A chart dir written as "./charts" therefore never matched "charts", so every changed chart was reported as "not a valid chart directory" and skipped, and --all found no charts at all while still reporting success. Clean the configured chart dir with filepath.Clean so equivalent spellings such as "./charts" and "charts/" match. Signed-off-by: Raphael Fakhri <153192858+RaphaelFakhri@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
LookupChartDircompares the cleaned parent of a candidate chart directory (filepath.Dir) with the configured chart directory, but only strips trailing slashes from the configured value. When--chart-dirs(orchart-dirsinct.yaml) is written as./charts, the comparison ischarts == ./charts, which never matches. As a result:ct list-changed,ct lintandct installprintDirectory "charts/foo" is not a valid chart directory. Skipping...for every changed chart and report no changes.ct lint --allfinds no charts and still printsAll charts linted successfully, so CI passes without linting anything.This change cleans the configured chart dir with
filepath.Clean, so equivalent spellings such as./chartsandcharts/match. It addsTestLookupChartDircovering the plain, trailing-slash and leading./forms.Reproduction on
main(repository withcharts/fooand a committed change tocharts/foo/values.yaml):With this change, both commands print
charts/foo.Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #877Special notes for your reviewer:
This is one cause of the "not a valid chart directory" reports in #226, #315 and #633, which were closed by the stale bot. Absolute chart dirs and Windows path separators are separate causes and are not addressed here.