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.

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

Server lacks SSH keys #4620

Closed
annasong20 opened this issue May 5, 2022 · 7 comments
Closed

Server lacks SSH keys #4620

annasong20 opened this issue May 5, 2022 · 7 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@annasong20
Copy link
Contributor

annasong20 commented May 5, 2022

Describe the bug
The server that runs the CI pipeline when users open a PR doesn't have ssh keys. We currently skip all ssh tests in remoteload_test.go because the server returns an error. Once the server has ssh keys, the server should produce the expected output for those tests.

Files that can reproduce the issue
remoteload_test.go contains ssh tests for which skip: true. If you delete this line and run these tests on the server, you will observe errors caused by the lack of ssh keys instead of the expected behavior.

Expected output
An example ssh test case is TestRemoteResourceSsh/scp_shorthand. The expected output is:

=== RUN   TestRemoteResourceSsh/scp_shorthand
--- PASS: TestRemoteResourceSsh/scp_shorthand

Actual output
For the same example test cases as above, the actual output due to the lack of ssh keys is:

=== RUN   TestRemoteResourceSsh/scp_shorthand
remoteload_test.go:70: 
           	Error Trace:	remoteload_test.go:70
           	            				remoteload_test.go:159
           	Error:      	Received unexpected error:
           	            	exit status 128
           	            	git cmd = '/usr/local/bin/git fetch --depth=1 origin v1.0.6'
           	            	sigs.k8s.io/kustomize/api/internal/git.gitRunner.run.func1
           	            		/Users/runner/work/kustomize/kustomize/api/internal/git/gitrunner.go:51
           	            	sigs.k8s.io/kustomize/api/internal/utils.TimedCall.func1
           	            		/Users/runner/work/kustomize/kustomize/api/internal/utils/timedcall.go:16
           	            	runtime.goexit
           	            		/Users/runner/hostedtoolcache/go/1.18.1/x64/src/runtime/asm_amd64.s:1571
           	            	accumulation err='accumulating resources from 'git@github.com:kubernetes-sigs/kustomize//examples/multibases/dev/?ref=v1.0.6': evalsymlink failure on '/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/kustomize-1472700803/git@github.com:kubernetes-sigs/kustomize/examples/multibases/dev/?ref=v1.0.6' : lstat /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/kustomize-1472700803/git@github.com:kubernetes-sigs: no such file or directory'
           	            	sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateResources
           	            		/Users/runner/work/kustomize/kustomize/api/internal/target/kusttarget.go:419
           	            	sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateTarget
           	            		/Users/runner/work/kustomize/kustomize/api/internal/target/kusttarget.go:189
           	            	sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).AccumulateTarget
           	            		/Users/runner/work/kustomize/kustomize/api/internal/target/kusttarget.go:182
           	            	sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).makeCustomizedResMap
           	            		/Users/runner/work/kustomize/kustomize/api/internal/target/kusttarget.go:128
           	            	sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).MakeCustomizedResMap
           	            		/Users/runner/work/kustomize/kustomize/api/internal/target/kusttarget.go:119
           	            	sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).Run
           	            		/Users/runner/work/kustomize/kustomize/api/krusty/kustomizer.go:89
           	            	sigs.k8s.io/kustomize/api/krusty_test.testRemoteResource
           	            		/Users/runner/work/kustomize/kustomize/api/krusty/remoteload_test.go:62
           	            	sigs.k8s.io/kustomize/api/krusty_test.TestRemoteResourceSsh.func1
           	            		/Users/runner/work/kustomize/kustomize/api/krusty/remoteload_test.go:159
           	            	testing.tRunner
           	            		/Users/runner/hostedtoolcache/go/1.18.1/x64/src/testing/testing.go:1439
           	            	runtime.goexit
           	            		/Users/runner/hostedtoolcache/go/1.18.1/x64/src/runtime/asm_amd64.s:1571
           	            	accumulating resources
           	            	sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).accumulateTarget
           	            		/Users/runner/work/kustomize/kustomize/api/internal/target/kusttarget.go:191
           	            	sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).AccumulateTarget
           	            		/Users/runner/work/kustomize/kustomize/api/internal/target/kusttarget.go:182
           	            	sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).makeCustomizedResMap
           	            		/Users/runner/work/kustomize/kustomize/api/internal/target/kusttarget.go:128
           	            	sigs.k8s.io/kustomize/api/internal/target.(*KustTarget).MakeCustomizedResMap
           	            		/Users/runner/work/kustomize/kustomize/api/internal/target/kusttarget.go:119
           	            	sigs.k8s.io/kustomize/api/krusty.(*Kustomizer).Run
           	            		/Users/runner/work/kustomize/kustomize/api/krusty/kustomizer.go:89
           	            	sigs.k8s.io/kustomize/api/krusty_test.testRemoteResource
           	            		/Users/runner/work/kustomize/kustomize/api/krusty/remoteload_test.go:62
           	            	sigs.k8s.io/kustomize/api/krusty_test.TestRemoteResourceSsh.func1
           	            		/Users/runner/work/kustomize/kustomize/api/krusty/remoteload_test.go:159
           	            	testing.tRunner
           	            		/Users/runner/hostedtoolcache/go/1.18.1/x64/src/testing/testing.go:1439
           	            	runtime.goexit
           	            		/Users/runner/hostedtoolcache/go/1.18.1/x64/src/runtime/asm_amd64.s:1571
           	Test:       	TestRemoteResourceSsh/scp_shorthand
   --- FAIL: TestRemoteResourceSsh/scp_shorthand (0.26s) 

Kustomize version
Master branch, which is currently at commit 17b42a9.

Platform
Linux and MacOS

@annasong20 annasong20 added the kind/bug Categorizes issue or PR as related to a bug. label May 5, 2022
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label May 5, 2022
@KnVerey
Copy link
Contributor

KnVerey commented May 11, 2022

/triage accepted

In the meantime, we should only skip the tests if we're on CI, since they should pass locally. It's not ideal, but we do run the tests locally as part of the release process, so we would still at least catch any issues at that stage.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 11, 2022
@mightyguava
Copy link
Contributor

mightyguava commented Aug 12, 2022

Summarizing some offline conversation with @KnVerey

Only ssh keys belonging to valid github users can clone from github, even for public repos. Deploy Key provides an alternative if one only need read-only access to a specific GitHub repo.

The tests in remote_test.go access only 2 repos, https://github.com/kubernetes-sigs/kustomize and https://github.com/annasong20/kustomize-test.

It should be enough to do the following:

  1. create an SSH key that is then added as a deploy key to both repos. The ssh key should then be able to clone both repos.
  2. add the private key as a secret to kustomize’s repo [docs].
  3. add the secret to the ssh_key field in the actions/checkout config for the github action. This involves modifying the checkout action to add an ssh_key field https://github.com/kubernetes-sigs/kustomize/blob/master/.github/workflows/go.yml#L25-L28. Something like the below, assuming the secret was named DEPLOY_KEY:
- name: Check out code into the Go module directory
  uses: actions/checkout@v2
  with:
     ssh-key: ${{ secrets.DEPLOY_KEY }}

Based on the docs, this should configure the git CLI to use this ssh key automatically.

[1] The docs didn't mention if the same private key can be used as the deploy key for multiple repos. I didn't test it out but it seems ok in theory.

@KnVerey
Copy link
Contributor

KnVerey commented Aug 12, 2022

Thanks for the investigation! I think that plan makes sense. I've created a read-only deploy key and added it as a secret named DEPLOY_SSH_KEY.

@mightyguava
Copy link
Contributor

mightyguava commented Aug 12, 2022

@KnVerey we also discussed removing anna's repo from the tests since we only want to clone this repo. I took a look.

It’s being used as an example of a repo that has kustomization.yaml at the root of the repo, where the repospec would not have a path component. I don’t know how important that is, but this code path can’t be exercised on the kustomize repo for obvious reasons. If we want to keep those tests, i’d recommend cloning her repo into somewhere that the team has co-ownership of, maybe kustomize-sigs.

@natasha41575
Copy link
Contributor

It’s being used as an example of a repo that has kustomization.yaml at the root of the repo, where the repospec would not have a path component. I don’t know how important that is, but this code path can’t be exercised on the kustomize repo for obvious reasons. If we want to keep those tests, i’d recommend cloning her repo into somewhere that the team has co-ownership of, maybe kustomize-sigs.

@mightyguava What's the name of the test? I'm wondering if it would be sufficient to have a unit test for repospec with an empty path, rather than need a full repo for whatever code that the test is covering.

@mightyguava
Copy link
Contributor

There are a few of them, here's one

"repo": {
kustomization: `
resources:
- https://github.com/annasong20/kustomize-test.git?ref=main`,
expected: multibaseDevExampleBuild,
},
. You can find the rest if you search annasong20 in that file.

@mightyguava
Copy link
Contributor

@annasong20 @KnVerey can this be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

5 participants