Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Resolve symlink for root directory and update tests #2473

Merged
merged 6 commits into from
Sep 3, 2021
Merged

Resolve symlink for root directory and update tests #2473

merged 6 commits into from
Sep 3, 2021

Conversation

phanimarupaka
Copy link
Contributor

@phanimarupaka phanimarupaka commented Aug 31, 2021 •
edited
Loading

Resolve the symlinks only for the root package directory on which kpt command is being invoked.
Update the existing tests.

➜  foo git:(main) kpt fn eval hello -i search-replace:v0.1 -- by-path='*.name'
[WARN] resolved symlink "hello" to "wordpress", please note that the symlinks within the package are ignored
[RUNNING] "gcr.io/kpt-fn/search-replace:v0.1"
[PASS] "gcr.io/kpt-fn/search-replace:v0.1"
  Results:
    [INFO] Matched field value "wordpress" in file "deployment/deployment.yaml" in field "metadata.name"
    [INFO] Matched field value "wp-pv-claim" in file "deployment/volume.yaml" in field "metadata.name"
    [INFO] Matched field value "wordpress-mysql" in file "mysql/deployment.yaml" in field "metadata.name"
    [INFO] Matched field value "mysql-pv-claim" in file "mysql/deployment.yaml" in field "metadata.name"
    ...(2 line(s) truncated, use '--truncate-output=false' to disable)

➜  foo git:(main) kpt fn render hello
[WARN] resolved symlink "hello" to "wordpress", please note that the symlinks within the package are ignored
Package "wordpress/mysql": 
[RUNNING] "gcr.io/kpt-fn/set-label:v0.1"
[PASS] "gcr.io/kpt-fn/set-label:v0.1"

Package "wordpress": 
[RUNNING] "gcr.io/kpt-fn/set-label:v0.1"
[PASS] "gcr.io/kpt-fn/set-label:v0.1"
[RUNNING] "gcr.io/kpt-fn/kubeval:v0.1"
[PASS] "gcr.io/kpt-fn/kubeval:v0.1"

Successfully executed 3 function(s) in 2 package(s).

@phanimarupaka phanimarupaka force-pushed the SupportSymlinks branch 3 times, most recently from 6195e6b to 6e7f5a5 Compare September 1, 2021 06:21
droot
droot reviewed Sep 1, 2021
View reviewed changes
Copy link
Contributor

@droot droot left a comment

Choose a reason for hiding this comment

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

Looks good to me. I have one request for the output of the command.

internal/util/argutil/argutil.go Outdated Show resolved Hide resolved
internal/util/argutil/argutil.go Show resolved Hide resolved
@phanimarupaka phanimarupaka changed the title [WIP] Resolve symlink for root directory and update tests Resolve symlink for root directory and update tests Sep 1, 2021
droot
droot approved these changes Sep 3, 2021
View reviewed changes
Copy link
Contributor

@droot droot left a comment

Choose a reason for hiding this comment

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

Pl. address a small nit, then ship it.

internal/util/argutil/argutil.go Outdated
f, err := os.Lstat(path)
if err == nil && f.Mode().Type() == os.ModeSymlink {
isSymlink = true
} else if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we can move this up at the top (without else)

internal/cmddiff/cmddiff_test.go Show resolved Hide resolved
@phanimarupaka phanimarupaka merged commit 6bae6f6 into kptdev:main Sep 3, 2021
@phanimarupaka phanimarupaka linked an issue Sep 4, 2021 that may be closed by this pull request
kpt doesn't work with symlinks #1544
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@droot droot droot approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

kpt doesn't work with symlinks
2 participants
@phanimarupaka @droot