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

AUTH-296: work out etcd certificates #975

Merged
merged 3 commits into from
Oct 26, 2022

Conversation

stlaz
Copy link
Member

@stlaz stlaz commented Sep 23, 2022

I was not yet able to learn what's important for etcd serving/peer certs so I'm going with minimal config where client certificates only follow the kube CN and O schemes, and serving certificates get their hostnames in DNS/IPs SANs.

The PR is rebased on top of #970 and can only merge once this one merges.

It's very likely that we may not see any failures in etcd because no peer communication is likely to happen with just one etcd instance. I'll need to test the cert setup with at least 2 etcds or ask someone with the necessary knowledge about how the peer certs should look like.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 23, 2022
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 27, 2022
@stlaz stlaz force-pushed the etcd_certs branch 2 times, most recently from d0df9ab to 44227dc Compare September 29, 2022 12:41
@stlaz stlaz changed the title WIP: AUTH-296: work out etcd certificates AUTH-296: work out etcd certificates Sep 29, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 29, 2022
@stlaz
Copy link
Member Author

stlaz commented Sep 29, 2022

This PR now follows the certificate configuration from https://github.com/stlaz/etcd-certs.

@stlaz
Copy link
Member Author

stlaz commented Sep 29, 2022

/test e2e-openshift-conformance-sig-node

Copy link
Contributor

@oglok oglok left a comment

Choose a reason for hiding this comment

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

What's the purpose of the .image_references empty file? What do we gain in terms of security by making this repo unimportable?

@stlaz
Copy link
Member Author

stlaz commented Oct 24, 2022

What's the purpose of the .image_references empty file? What do we gain in terms of security by making this repo unimportable?

I do not know its purpose. The repository is currently unimportable because the filename is malformed, the commit is fixing it.

@pmtk
Copy link
Member

pmtk commented Oct 25, 2022

/retest

@oglok
Copy link
Contributor

oglok commented Oct 26, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2022
The dot at the end of the filename makes this repository
unimportable from outside
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2022
@stlaz
Copy link
Member Author

stlaz commented Oct 26, 2022

somehow a lonely ultimateTrustBundle variable appeared in the code, removed now, all tests should hopefully pass again 🙂

@stlaz
Copy link
Member Author

stlaz commented Oct 26, 2022

/test periodic-ocp-4.13-images

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 26, 2022

@stlaz: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/periodic-ocp-4.13-images f147665 link true /test periodic-ocp-4.13-images

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@stlaz
Copy link
Member Author

stlaz commented Oct 26, 2022

/retest

@stlaz
Copy link
Member Author

stlaz commented Oct 26, 2022

@oglok I think this might be good for re-tagging 🙂

@oglok
Copy link
Contributor

oglok commented Oct 26, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 26, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oglok, stlaz

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:

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

@openshift-merge-robot openshift-merge-robot merged commit b9ae777 into openshift:main Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants