Skip to content

Additional trust bundle #1159

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
Merged
openshift-merge-robot merged 1 commit into from
Dec 2, 2020
Merged

Additional trust bundle #1159

openshift-merge-robot merged 1 commit into from
Dec 2, 2020

Conversation

@Avielyo10
Copy link
Contributor

@Avielyo10 Avielyo10 commented Nov 22, 2020

No description provided.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 22, 2020

Hi @Avielyo10. Thanks for your PR.

I'm waiting for a openshift-metal3 member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 22, 2020
@Avielyo10
Copy link
Contributor Author

Avielyo10 commented Nov 22, 2020

@zaneb According to your CR, this also fix a bug caused when passing the ca through env vars

@Avielyo10
Copy link
Contributor Author

Avielyo10 commented Nov 23, 2020

@hardys @zaneb

@hardys
Copy link

hardys commented Nov 23, 2020

Looks like you need to rebase this on latest master since #1152 merged?

@Avielyo10
Copy link
Contributor Author

Avielyo10 commented Nov 23, 2020

@hardys I merged

@hardys
Copy link

hardys commented Nov 23, 2020

@hardys I merged

Please rebase so we can review only the new changes e.g

git checkout master
git pull
git checkout additional-trust-bundle
git rebase -i origin

zaneb
zaneb reviewed Nov 23, 2020
View reviewed changes
ocp_install_env.sh Outdated
if [ -z $MIRROR_IMAGES ]; then
echo "additionalTrustBundle: |"
fi
cat << EOF
Copy link
Member

@zaneb zaneb Nov 23, 2020

Choose a reason for hiding this comment

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

This cat is unnecessary, it makes this really hard to read for no reason.

Actually the other one isn't necessary either. All you need is:

awk '{ print " ", $0 }' "${ADDITIONAL_TRUST_BUNDLE}"

Copy link
Contributor Author

@Avielyo10 Avielyo10 Nov 25, 2020

Choose a reason for hiding this comment

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

Done

@Avielyo10 Avielyo10 force-pushed the additional-trust-bundle branch from 542d72d to d08eb96 Compare November 24, 2020 15:05
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 24, 2020
@Avielyo10 Avielyo10 force-pushed the additional-trust-bundle branch from d08eb96 to b93743f Compare November 25, 2020 07:09
@Avielyo10
Copy link
Contributor Author

Avielyo10 commented Nov 25, 2020

@hardys @zaneb done.

@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 25, 2020
@Avielyo10 Avielyo10 requested a review from zaneb November 25, 2020 08:53
@Avielyo10
Copy link
Contributor Author

Avielyo10 commented Nov 30, 2020

@hardys @zaneb Can someone take a look?

@Avielyo10
Copy link
Contributor Author

Avielyo10 commented Dec 2, 2020

@zaneb @hardys @vrutkovs

@hardys
Copy link

hardys commented Dec 2, 2020

@zaneb @hardys @vrutkovs

Please can you rebase and squash the commits like I previously asked?

@Avielyo10 Avielyo10 force-pushed the additional-trust-bundle branch from d619848 to 3c41002 Compare December 2, 2020 11:32
@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 2, 2020
@Avielyo10 Avielyo10 force-pushed the additional-trust-bundle branch from 58908e0 to 3c41002 Compare December 2, 2020 11:40
@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 2, 2020
Additional trust bundle - fix
0ecfed7 
* Simplify additional_trust_bundle()
* Pass ca via path instead of via env vars
* Improve readability
@Avielyo10 Avielyo10 force-pushed the additional-trust-bundle branch from a77fd66 to 0ecfed7 Compare December 2, 2020 12:04
@Avielyo10
Copy link
Contributor Author

Avielyo10 commented Dec 2, 2020

@hardys done

@hardys
Copy link

hardys commented Dec 2, 2020

/ok-to-test
/approve

Thanks!

@openshift-ci-robot openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 2, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 2, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hardys

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 2, 2020
@hardys
Copy link

hardys commented Dec 2, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2020
@openshift-merge-robot openshift-merge-robot merged commit 1b6768f into openshift-metal3:master Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vrutkovs vrutkovs Awaiting requested review from vrutkovs

@zaneb zaneb Awaiting requested review from zaneb

Assignees

@hardys hardys

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Avielyo10 @openshift-ci-robot @hardys @zaneb @openshift-merge-robot