Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Add --tls-skip-verify global option affecting outgoing HTTP requests#831

Merged
shomron merged 1 commit intoksonnet:masterfrom
shomron:issue-726-tls-skip-verify
Aug 15, 2018
Merged

Add --tls-skip-verify global option affecting outgoing HTTP requests#831
shomron merged 1 commit intoksonnet:masterfrom
shomron:issue-726-tls-skip-verify

Conversation

@shomron
Copy link
Collaborator

@shomron shomron commented Aug 14, 2018 •
edited
Loading

This change allows disabling TLS certification verification when interacting with GitHub/Helm registries. This is not recommended and is only provided as a option when working around trust issues with corporate proxies.

Manual testing

Install and run mitmproxy:

brew install mitmproxy
mitmproxy

Test registry / pkg commands via proxy, with and without verification

# registry add - fail
https_proxy=localhost:8080 ks registry add helm-stable https://kubernetes-charts.storage.googleapis.com
https_proxy=localhost:8080 ks registry add foo github.com/ksonnet/parts/tree/master/incubator

# registry add - success
https_proxy=localhost:8080 ks registry add helm-stable https://kubernetes-charts.storage.googleapis.com --tls-skip-verify
https_proxy=localhost:8080 ks registry add foo github.com/ksonnet/parts/tree/master/incubator --tls-skip-verify

# pkg list - fail
https_proxy=localhost:8080 ks pkg list

# pkg list - success
https_proxy=localhost:8080 ks pkg list --tls-skip-verify

# pkg install - fail
https_proxy=localhost:8080 ks pkg install incubator/nginx
https_proxy=localhost:8080 ks pkg install helm-stable/redis

# pkg install - success
https_proxy=localhost:8080 ks pkg install incubator/nginx --tls-skip-verify
https_proxy=localhost:8080 ks pkg install helm-stable/redis --tls-skip-verify

Signed-off-by: Oren Shomron shomron@gmail.com

@shomron
Add --tls-skip-verify global option affecting outgoing HTTP requests
de033c2 
Signed-off-by: Oren Shomron <shomron@gmail.com>
@coveralls
Copy link

coveralls commented Aug 14, 2018

Pull Request Test Coverage Report for Build 1280

  • 139 of 174 (79.89%) changed or added relevant lines in 33 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.1%) to 71.096%
Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/registry/manager.go 5 6 83.33%
pkg/actions/pkg_install.go 4 6 66.67%
pkg/actions/pkg_list.go 7 9 77.78%
pkg/actions/registry_list.go 2 4 50.0%
pkg/util/github/github.go 11 13 84.62%
pkg/actions/registry_set.go 3 7 42.86%
pkg/actions/registry_describe.go 3 7 42.86%
pkg/registry/github.go 2 8 25.0%
pkg/registry/package_manager.go 3 15 20.0%
Files with Coverage Reduction New Missed Lines %
pkg/actions/registry_set.go 1 51.72%
Totals Coverage Status
Change from base Build 1279: 0.1%
Covered Lines: 12134
Relevant Lines: 17067
💛 - Coveralls

@shomron
Copy link
Collaborator Author

shomron commented Aug 14, 2018 •
edited
Loading

I missed ks init - will push an update.

@shomron
Copy link
Collaborator Author

shomron commented Aug 15, 2018

@bryanl I'm going to merge this as-is, and continue the init work in a new issue. This change alone has value if one needed to work around network constraints when deploying an existing app.

@shomron shomron merged commit c7b94fe into ksonnet:master Aug 15, 2018
@shomron shomron deleted the issue-726-tls-skip-verify branch August 15, 2018 12:06
@bryanl
Copy link
Member

bryanl commented Aug 15, 2018

@shomron This is fine, you always have the recourse of doing ks init foo --skip-default-registries

@shomron
Copy link
Collaborator Author

shomron commented Aug 15, 2018

Actually, there remains an issue with fetching the OpenAPI spec for ksonnnet-lib generation, so I think that we'll still need to account for that.

@shomron shomron mentioned this pull request Aug 15, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@bryanl bryanl bryanl approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shomron @coveralls @bryanl