Skip to content

fix: edit original file if the given skaffold path is a symlink - #8955

Merged
renzodavid9 merged 3 commits into
GoogleContainerTools:mainfrom
alphanota:fix-symlink
Jul 21, 2023
Merged

fix: edit original file if the given skaffold path is a symlink#8955
renzodavid9 merged 3 commits into
GoogleContainerTools:mainfrom
alphanota:fix-symlink

Conversation

@alphanota

Copy link
Copy Markdown
Contributor

Fixes: #8489
Related: Relevant tracking issues, for context
Merge before/after: Dependent or prerequisite PRs

Description

This PR fixes issue where skaffold fix --overwrite command doesn't overwrite the contents of a symlink

First we make a backup of original file by copying the contents of symlink to new file (in same directory as symlink, if applicable)

Then we overwrite config file with new contents

This also accounts for case where symlink of symlink is provided.

User facing changes (remove if N/A)

Setup:
$ $HOME/skaffold/out/skaffold fix --overwrite
Backed up previous skaffold.yaml at skaffold.yaml.v2
New config at version skaffold/v4beta6 generated and written to skaffold.yaml

@renzodavid9
renzodavid9 self-requested a review July 19, 2023 14:35
Comment thread cmd/skaffold/app/cmd/fix.go Outdated
Comment thread cmd/skaffold/app/cmd/fix.go Outdated
Comment thread cmd/skaffold/app/cmd/fix.go Outdated
Comment thread cmd/skaffold/app/cmd/fix.go
Comment thread cmd/skaffold/app/cmd/fix.go Outdated
@renzodavid9

Copy link
Copy Markdown
Contributor

Hey @alphanota, thanks for the PR. Could you please change the PR's title and the commit message to follow the conventionalcommits.org? We have a linter that checks that. Something like the following will work:

fix: edit original file if the given skaffold path is a symlink

Thanks!

@alphanota alphanota changed the title Fix symlink fix: edit original file if the given skaffold path is a symlink Jul 19, 2023
@codecov

codecov Bot commented Jul 20, 2023

Copy link
Copy Markdown

Codecov Report

Merging #8955 (20e1494) into main (290280e) will decrease coverage by 6.83%.
The diff coverage is 49.80%.

@@            Coverage Diff             @@
##             main    #8955      +/-   ##
==========================================
- Coverage   70.48%   63.66%   -6.83%     
==========================================
  Files         515      624     +109     
  Lines       23150    31926    +8776     
==========================================
+ Hits        16317    20325    +4008     
- Misses       5776    10076    +4300     
- Partials     1057     1525     +468     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/completion.go 13.04% <0.00%> (-1.25%) ⬇️
cmd/skaffold/app/cmd/config/list.go 65.21% <ø> (ø)
cmd/skaffold/app/cmd/config/set.go 88.72% <ø> (ø)
cmd/skaffold/app/cmd/config/util.go 54.28% <ø> (ø)
cmd/skaffold/app/cmd/credits.go 100.00% <ø> (ø)
cmd/skaffold/app/cmd/credits/export.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go 40.90% <0.00%> (-12.94%) ⬇️
cmd/skaffold/app/cmd/generate_pipeline.go 60.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_modules.go 65.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_profiles.go 66.66% <ø> (ø)
... and 40 more

... and 415 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@renzodavid9 renzodavid9 added the kokoro:force-run forces a kokoro re-run on a PR label Jul 20, 2023
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Jul 20, 2023
@renzodavid9 renzodavid9 added the kokoro:force-run forces a kokoro re-run on a PR label Jul 20, 2023
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Jul 20, 2023
@renzodavid9

Copy link
Copy Markdown
Contributor

LGTM, let's wait for the tests to finish before merging this. Thanks!

@renzodavid9 renzodavid9 added the kokoro:force-run forces a kokoro re-run on a PR label Jul 21, 2023
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Jul 21, 2023
@renzodavid9 renzodavid9 added the kokoro:force-run forces a kokoro re-run on a PR label Jul 21, 2023
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Jul 21, 2023
@renzodavid9
renzodavid9 merged commit 3114e3f into GoogleContainerTools:main Jul 21, 2023
@alphanota
alphanota deleted the fix-symlink branch January 14, 2025 21:29
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.

skaffold fix --overwrite does not overwrite, instead it renames.

4 participants