Secure registry support #3516

GeekArthur · 2020-07-08T16:38:25Z

Signed-off-by: jingfu wang jingfu.j.wang@ibm.com

What type of PR is this?
/kind feature
/area devfile

What does does this PR do / why we need it:
The PR implements the secure registry feature which user can use odo to interact with private/secure registry.
Design proposal: #3329

Which issue(s) this PR fixes:
Fixes #2893

**How to test changes:

Create a personal access token with repo access scope from your GitHub account
Run odo registry add <registry name> <registry URL> --token <token> to add secure registry
Run odo catalog list components to list available components from secure registry
Run odo create <component type> --registry <registry name> to create component from secure registry
Run odo registry delete <registry name> to delete secure registry
Note: you can also test odo registry update <registry name> <registry URL> --token <token> to update secure registry once you already create the registry

Special notes to the reviewer:

Currently only support token for authentication/authorization as there is no use case to use username/password
All platform tests are passed, currently it support MacOS, Linux and Windows, keyring implementation depends on the platform
Cluster-hosted registry for CP4A is not completed as the testing environment is not ready
Regarding test automation, we should have a mechanism to store the token so that we don't expose the token to public, need to confirm if the existing testing infrastructure support that, test automation will fully complete once it's confirmed.

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

johnmcollier

Left some review comments.

Could you also update Odo's documentation to add instructions for the following:

Adding a secured registry
Github token requirements (what permissions are needed)
Keyring setup (esp. on Linux)

glide.yaml

pkg/catalog/catalog.go

pkg/odo/cli/component/create.go

pkg/util/util.go

johnmcollier · 2020-07-08T18:06:47Z

@GeekArthur As a heads up, the DownloadFile unit tests are failing in the CI:

 ok  	github.com/openshift/odo/pkg/sync	1.089s
ok  	github.com/openshift/odo/pkg/testingutil	1.026s
?   	github.com/openshift/odo/pkg/testingutil/filesystem	[no test files]
ok  	github.com/openshift/odo/pkg/url	1.562s
ok  	github.com/openshift/odo/pkg/url/labels	1.010s
--- FAIL: TestDownloadFile (0.00s)
    --- FAIL: TestDownloadFile/Case_2:_Input_url_is_invalid (0.00s)
        util_test.go:1376: Got: [], want: []
        util_test.go:1382: Failed to delete file with error remove ./test.yaml: no such file or directory
    --- FAIL: TestDownloadFile/Case_3:_Input_url_is_an_empty_string (0.00s)
        util_test.go:1372: Failed to read file with error open ./test.yaml: no such file or directory
        util_test.go:1376: Got: [], want: []
        util_test.go:1382: Failed to delete file with error remove ./test.yaml: no such file or directory
FAIL
FAIL	github.com/openshift/odo/pkg/util	0.406s
?   	github.com/openshift/odo/pkg/version	[no test files]
ok  	github.com/openshift/odo/pkg/watch	31.144s
make[1]: *** [test] Error 1
make[1]: Leaving directory `/go/src/github.com/openshift/odo'
make: *** [openshiftci-presubmit-unittests] Error 2
2020/07/08 16:47:01 Container test in pod unit failed, exit code 2, reason Error
2020/07/08 16:47:01 No custom metadata found and prow metadata already exists. Not updating the metadata.
2020/07/08 16:47:02 Ran for 8m15s
error: some steps failed:
  * could not run steps: step unit failed: test "unit" failed: the pod ci-op-srt8qf8w/unit failed after 4m39s (failed containers: test): ContainerFailed one or more containers exited
Container test exited with code 2, reason Error
---
enshift/odo/pkg/odo/cli/ui	[no test files]
ok  	github.com/openshift/odo/pkg/sync	1.089s
ok  	github.com/openshift/odo/pkg/testingutil	1.026s
?   	github.com/openshift/odo/pkg/testingutil/filesystem	[no test files]
ok  	github.com/openshift/odo/pkg/url	1.562s
ok  	github.com/openshift/odo/pkg/url/labels	1.010s
--- FAIL: TestDownloadFile (0.00s)
    --- FAIL: TestDownloadFile/Case_2:_Input_url_is_invalid (0.00s)
        util_test.go:1376: Got: [], want: []
        util_test.go:1382: Failed to delete file with error remove ./test.yaml: no such file or directory
    --- FAIL: TestDownloadFile/Case_3:_Input_url_is_an_empty_string (0.00s)
        util_test.go:1372: Failed to read file with error open ./test.yaml: no such file or directory
        util_test.go:1376: Got: [], want: []
        util_test.go:1382: Failed to delete file with error remove ./test.yaml: no such file or directory
FAIL
FAIL	github.com/openshift/odo/pkg/util	0.406s
?   	github.com/openshift/odo/pkg/version	[no test files]
ok  	github.com/openshift/odo/pkg/watch	31.144s
make[1]: *** [test] Error 1
make[1]: Leaving directory `/go/src/github.com/openshift/odo'
make: *** [openshiftci-presubmit-unittests] Error 2
---
time="2020-07-08T16:47:02Z" level=info msg="Reporting job state \"failed\" with reason \"executing_graph:step_failed:running_pod\""

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

GeekArthur · 2020-07-08T20:44:39Z

Left some review comments.

Could you also update Odo's documentation to add instructions for the following:
* Adding a secured registry

* Github token requirements (what permissions are needed)

* Keyring setup (esp. on Linux)

Thanks @johnmcollier for reviewing this PR quickly, I will update the corresponding instructions for odo docs in another PR. I will push commits to fix the tests and address all comments that I can resolve, and leave the open questions as unresolved comments.

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

GeekArthur · 2020-07-09T18:34:51Z

@johnmcollier All comments are addressed, please review again.

The following things still need to finish:

Test automation: the existing test infrastructure needs a mechanism to store and retrieve credential for testing secure registry through configuring Travis CI and Openshift CI, @amitkrout is working on that.
CP4A cluster-hosted registry test: we already schedule a meeting (July 13) with CP4A folks to discuss the design and the way of testing.
Documentation
1. Adding a secured registry
2. Github token requirements (what permissions are needed)
3. Keyring setup (esp. on Linux)

GeekArthur · 2020-07-09T19:01:19Z

/assign

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

GeekArthur · 2020-07-09T19:35:35Z

/retest

johnmcollier

Overall changes look good to me, since we'll be handling the docs in a separate PR.

Left a couple more comments, mostly nits.

pkg/catalog/catalog.go

pkg/odo/cli/component/create.go

pkg/odo/cli/registry/util/util.go

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

GeekArthur · 2020-07-13T23:07:46Z

@johnmcollier Comments addressed, please review again.

johnmcollier

/lgtm

kadel · 2020-07-14T09:28:34Z

/approve

openshift-ci-robot · 2020-07-14T09:28:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [kadel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2020-07-14T09:41:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-07-14T09:53:05Z

/retest

Please review the full test history for this PR and help us cut down flakes.

codecov · 2020-07-14T11:58:19Z

Codecov Report

Merging #3516 into master will decrease coverage by 0.19%.
The diff coverage is 50.90%.

@@            Coverage Diff             @@
##           master    #3516      +/-   ##
==========================================
- Coverage   46.47%   46.27%   -0.20%     
==========================================
  Files         112      113       +1     
  Lines       11244    11420     +176     
==========================================
+ Hits         5226     5285      +59     
- Misses       5513     5622     +109     
- Partials      505      513       +8

Impacted Files	Coverage Δ
pkg/catalog/catalog.go	`54.77% <33.33%> (-0.60%)`	⬇️
pkg/util/util.go	`51.98% <33.33%> (-3.37%)`	⬇️
pkg/preference/preference.go	`62.50% <69.23%> (+0.62%)`	⬆️
pkg/odo/cli/registry/util/util.go	`75.00% <75.00%> (ø)`
pkg/devfile/adapters/docker/component/adapter.go	`64.51% <0.00%> (-9.50%)`	⬇️
pkg/kclient/pods.go	`38.13% <0.00%> (-6.42%)`	⬇️
pkg/lclient/containers.go	`69.47% <0.00%> (-2.95%)`	⬇️
pkg/lclient/mock_client.go	`14.11% <0.00%> (-0.83%)`	⬇️
pkg/lclient/client.go	`0.00% <0.00%> (ø)`
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26ea4de...53a60c6. Read the comment docs.

feat: secure registry support

4623300

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

openshift-ci-robot added kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. labels Jul 8, 2020

openshift-ci-robot requested review from dharmit and maysunfaisal July 8, 2020 16:38

johnmcollier suggested changes Jul 8, 2020

View reviewed changes

openshift-ci-robot assigned johnmcollier Jul 8, 2020

test: fix TestDownloadFile test

348d155

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

GeekArthur added 4 commits July 8, 2020 17:30

docs: update error message

09deaa3

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

refactor: move credential prefix to util package

a01f8e3

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

feat: display and store registry type

8bc34e4

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

refactor: remove optional argumetns

4d89d42

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

openshift-ci-robot assigned GeekArthur Jul 9, 2020

fix: unit test

4b7cceb

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

johnmcollier suggested changes Jul 9, 2020

View reviewed changes

pkg/catalog/catalog.go Outdated Show resolved Hide resolved

pkg/odo/cli/component/create.go Outdated Show resolved Hide resolved

pkg/odo/cli/registry/util/util.go Show resolved Hide resolved

refactor: add unit test and refactor

53a60c6

Signed-off-by: jingfu wang <jingfu.j.wang@ibm.com>

johnmcollier approved these changes Jul 14, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jul 14, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Jul 14, 2020

openshift-merge-robot merged commit 86fa18d into redhat-developer:master Jul 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Secure registry support #3516

Secure registry support #3516

GeekArthur commented Jul 8, 2020 •

edited

Loading

johnmcollier left a comment

johnmcollier commented Jul 8, 2020

GeekArthur commented Jul 8, 2020

GeekArthur commented Jul 9, 2020

GeekArthur commented Jul 9, 2020

GeekArthur commented Jul 9, 2020

johnmcollier left a comment

GeekArthur commented Jul 13, 2020

johnmcollier left a comment

kadel commented Jul 14, 2020

openshift-ci-robot commented Jul 14, 2020

openshift-bot commented Jul 14, 2020

openshift-bot commented Jul 14, 2020

codecov bot commented Jul 14, 2020 •

edited

Loading

Secure registry support #3516

Secure registry support #3516

Conversation

GeekArthur commented Jul 8, 2020 • edited Loading

johnmcollier left a comment

Choose a reason for hiding this comment

johnmcollier commented Jul 8, 2020

GeekArthur commented Jul 8, 2020

GeekArthur commented Jul 9, 2020

GeekArthur commented Jul 9, 2020

GeekArthur commented Jul 9, 2020

johnmcollier left a comment

Choose a reason for hiding this comment

GeekArthur commented Jul 13, 2020

johnmcollier left a comment

Choose a reason for hiding this comment

kadel commented Jul 14, 2020

openshift-ci-robot commented Jul 14, 2020

openshift-bot commented Jul 14, 2020

openshift-bot commented Jul 14, 2020

codecov bot commented Jul 14, 2020 • edited Loading

Codecov Report

GeekArthur commented Jul 8, 2020 •

edited

Loading

codecov bot commented Jul 14, 2020 •

edited

Loading