-
Notifications
You must be signed in to change notification settings - Fork 90
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
LOG-1076: Opt-in cert generation in EO #727
LOG-1076: Opt-in cert generation in EO #727
Conversation
/hold |
c372cb4
to
1acca09
Compare
78a31d2
to
9c6951b
Compare
/hold cancel |
/hold |
/test e2e-operator |
/retest |
2 similar comments
/retest |
/retest |
/hold |
0d61229
to
8ab9d2c
Compare
8ab9d2c
to
14b21c0
Compare
/hold cancel |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really awesome. Will give it an LGTM now. We can fix nits on a follow.
if certOK { | ||
decodedCert, err := pemDecodeCert(certBytes) | ||
if err != nil { | ||
return nil, err | ||
} | ||
x509Cert = decodedCert | ||
} else { | ||
return nil, fmt.Errorf("missing cert key from secret") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit. Suggest to skip one level of indentation like:
if !certOK {
return nil, fmt.Errorf("missing cert key from secret")
}
decodedCert, err := pemDecodeCert(certBytes)
if err != nil {
return nil, err
}
x509Cert = decodedCert
Same applies below for keyOk
and SerialOk
.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ewolinetz, periklis 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 |
cc @rubenvp8510 |
Description
For EO 5.2, adding the ability to opt-in for cert generation and management within EO.
Implementation of enhancement proposal (linked below).
Will also require OAL change to be merged in first since DN Organization and OrganizationUnit were adjusted to read correctly as part of this code change.
To opt in, annotate the
es
CR with:To have EO provide a tls cert/key ca-bundle for your component also annotate with:
or
/cc @jcantrill @periklis @jpkrohling
Links