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

Prefer secure connection during image pruning #14114

Merged
merged 2 commits into from
May 30, 2017
Merged

Prefer secure connection during image pruning #14114

merged 2 commits into from
May 30, 2017

Conversation

miminar
Copy link

@miminar miminar commented May 9, 2017

The default stays the same. When a CA bundle or a registry url is specified, require secure connection with certificate verification. Allow the user to force insecure connection using --force-insecure if he has to.

@miminar miminar requested a review from soltysh May 9, 2017 15:32
@miminar miminar force-pushed the secure-images-prune-by-default branch from ec98511 to ad4a823 Compare May 9, 2017 15:41
@miminar
Copy link
Author

miminar commented May 10, 2017

Regenerated completions.

@miminar
Copy link
Author

miminar commented May 10, 2017

Flake #14122

soltysh
soltysh approved these changes May 11, 2017
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

LGTM

@miminar
Copy link
Author

miminar commented May 11, 2017

Flake #12927

@miminar
Copy link
Author

miminar commented May 11, 2017

[test]

@miminar
Copy link
Author

miminar commented May 11, 2017

Flakse #14140. re-[test]

@soltysh
Copy link
Contributor

soltysh commented May 12, 2017

@miminar can you confirm if this PR will solve the problem from https://bugzilla.redhat.com/show_bug.cgi?id=1448595 ?

@miminar
Copy link
Author

miminar commented May 15, 2017

Flake #13426 in job https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1349/. re-[test]

@miminar
Copy link
Author

miminar commented May 15, 2017

@miminar can you confirm if this PR will solve the problem from https://bugzilla.redhat.com/show_bug.cgi?id=1448595 ?

If they keep using wrong certificate authority, this PR will at least allow them to force insecure connection.

@miminar
Copy link
Author

miminar commented May 15, 2017

Flake #12251. re-[test]

@miminar
Copy link
Author

miminar commented May 16, 2017

Test flake #14214. @soltysh all the sub tests passed individually in various runs. There was just not a single run where they passed all at once. May I request your merge powers?

@soltysh
Copy link
Contributor

soltysh commented May 17, 2017

[merge]

@miminar
Copy link
Author

miminar commented May 17, 2017

Flake #14228.

@miminar
Copy link
Author

miminar commented May 17, 2017 •
edited
Loading

Do not merge. I've just found another shortcoming.

@miminar miminar force-pushed the secure-images-prune-by-default branch from f023bc9 to 755f74e Compare May 17, 2017 16:07
@miminar
Copy link
Author

miminar commented May 17, 2017

Fixed. The code now takes into account an insecure flag in user's kubeconfig - just like before. Also the registry pinger now generates more readable errors. It won't attempt HTTP ping if a secure transport is requested.

@miminar
Copy link
Author

miminar commented May 17, 2017

@soltysh may I request one more look of yours?

@soltysh
Copy link
Contributor

soltysh commented May 17, 2017

[test]

@soltysh
Copy link
Contributor

soltysh commented May 17, 2017

flake #8571
[test]

@miminar
Copy link
Author

miminar commented May 18, 2017

The extended tests may need some changes. I'll investigate on Monday.

@miminar miminar force-pushed the secure-images-prune-by-default branch from 755f74e to a278e5d Compare May 24, 2017 14:22
@miminar
Copy link
Author

miminar commented May 24, 2017

The pruner now allows insecure connections for private addresses.

@miminar miminar force-pushed the secure-images-prune-by-default branch from a278e5d to 48ee2df Compare May 24, 2017 14:58
Prefer secure connection during image pruning
ed48ccd 
The default stays the same. When a CA bundle or a registry url is
specified, require secure connection with certificate verification.
Allow the user to force insecure connection using --force-insecure if he
has to.

Signed-off-by: Michal Minář <[email protected]>
@miminar miminar force-pushed the secure-images-prune-by-default branch from 48ee2df to af35aba Compare May 24, 2017 15:00
@miminar miminar mentioned this pull request May 24, 2017
Merged
@miminar
Copy link
Author

miminar commented May 24, 2017

Here's a documentation: openshift/openshift-docs#4471

@dmage could you please help review this? I'd like to get it in before the pruning changes for the read-only mode.

@miminar miminar force-pushed the secure-images-prune-by-default branch from af35aba to d8ea2a7 Compare May 26, 2017 10:22
@miminar
Copy link
Author

miminar commented May 26, 2017

No change, just re-signed the last commit to trigger new test runs.

@miminar
Copy link
Author

miminar commented May 26, 2017

Oh the extended test has been renamed. One more try:
[testextended][extended:core(ImagePrune)]

Updated generated completions
50b0858 
Signed-off-by: Michal Minář <[email protected]>
@miminar miminar force-pushed the secure-images-prune-by-default branch from d8ea2a7 to 50b0858 Compare May 26, 2017 13:27
@openshift-bot
Copy link
Contributor

Evaluated for origin testextended up to 50b0858

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 50b0858

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/508/) (Base Commit: a1dffba) (Extended Tests: core(ImagePrune))

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1794/) (Base Commit: a1dffba)

@miminar
Copy link
Author

miminar commented May 29, 2017

@mfojtik merge?

@mfojtik
Copy link
Contributor

mfojtik commented May 29, 2017

[merge][severity:blocker]

blocker bug: https://bugzilla.redhat.com/show_bug.cgi?id=1448595

@miminar
Copy link
Author

miminar commented May 29, 2017

Flake #12072

@mfojtik
Copy link
Contributor

mfojtik commented May 30, 2017

[merge][severity:blocker]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 50b0858

@openshift-bot
Copy link
Contributor

openshift-bot commented May 30, 2017 •
edited
Loading

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/838/) (Base Commit: 5a19120) (Extended Tests: blocker) (Image: devenv-rhel7_6281)

@miminar miminar mentioned this pull request May 30, 2017
Merged
@openshift-bot openshift-bot merged commit 39664e5 into openshift:master May 30, 2017
@miminar miminar mentioned this pull request Jun 27, 2017
Merged
@miminar miminar deleted the secure-images-prune-by-default branch July 25, 2017 14:59
openshift-merge-robot added a commit that referenced this pull request Aug 17, 2017
@openshift-merge-robot
Merge pull request #14914 from miminar/secure-image-prune
86abb1f 
Automatic merge from submit-queue

Image pruner: Determine protocol just once

Determine the registry protocol once. Do not change to other protocol during the run. This will produce nicer output without unrelated protocol fallback errors.

A follow-up for #14114 and #14405
Resolves #15291
@miminar miminar mentioned this pull request Sep 5, 2017
Merged
openshift-merge-robot added a commit that referenced this pull request Sep 14, 2017
@openshift-merge-robot
Merge pull request #16138 from miminar/origin3.6-secure-image-prune
afb8ebb 
Automatic merge from submit-queue

[3.6][Backport] Prune images (not)securely

Back-porting:
- #14114 
- #14405
- #14914
- #15899

Resolves [bz#1476779](https://bugzilla.redhat.com/show_bug.cgi?id=1476779)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soltysh soltysh soltysh 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.
4 participants
@miminar @soltysh @openshift-bot @mfojtik