[WFCORE-4362] Make the certificate authority used by a certificate-au… #192
Conversation
elytron/WFCORE-4362-configurable-certificate-authority-in-certificate-authority-account.adoc
Show resolved
Hide resolved
elytron/WFCORE-4362-configurable-certificate-authority-in-certificate-authority-account.adoc
Outdated
Show resolved
Hide resolved
| In the Elytron subsystem, LetsEncrypt is a value of CertificateAuthority enum. This enum contains name, url and staging url. | ||
| We can add new `certificate-authority` resource to the elytron | ||
| subsystem with non-optional attributes `name`, `production-url` and optional attribute `staging-url`. | ||
| This way, during the configuration of `certificate-authority-account` |
There was a problem hiding this comment.
Making the staging-url optional is ok. However, when implementing this, just double check the ACME related code in both Core and Elytron to see if any additional null checks should be added to avoid NPEs since with Let's Encrypt, we were able to assume both urls would be non-null.
There was a problem hiding this comment.
@fjuma Looking at the code and you are right, it will be necessary to add null checks on a few places. I will keep this in mind during implementation. Thank you!
elytron/WFCORE-4362-configurable-certificate-authority-in-certificate-authority-account.adoc
Outdated
Show resolved
Hide resolved
elytron/WFCORE-4362-configurable-certificate-authority-in-certificate-authority-account.adoc
Outdated
Show resolved
Hide resolved
elytron/WFCORE-4362-configurable-certificate-authority-in-certificate-authority-account.adoc
Outdated
Show resolved
Hide resolved
elytron/WFCORE-4362-configurable-certificate-authority-in-certificate-authority-account.adoc
Outdated
Show resolved
Hide resolved
|
|
||
| == Test Plan | ||
|
|
||
| Functionality tests added to Elytron testsuite. |
There was a problem hiding this comment.
Just a minor clarification, the functionality tests will be added to the Elytron tests in the Core testsuite, right?
There was a problem hiding this comment.
@fjuma yes, I specified tests plan more.
|
|
||
| === QE Contacts | ||
|
|
||
| * mailto:mchoma@redhat.com[Martin Choma] |
There was a problem hiding this comment.
@Skyllarr Other parts of analysis LGTM, so I will approve it once this will be changed ...
|
|
||
| Currently, `certificate-authority-account` contains optional attribute `certificate-authority` of type STRING. | ||
| The default and only allowed value is "LetsEncrypt". | ||
| We should be able to specify staging URL and production URL without breaking backwards compatibility. |
|
|
||
| In the Elytron subsystem, LetsEncrypt is a value of CertificateAuthority enum. This enum contains name, url and staging url. | ||
| We can add new `certificate-authority` resource to the elytron | ||
| subsystem with non-optional attributes `name`, `url` and optional attribute `staging-url`. |
There was a problem hiding this comment.
Just a minor comment: name is probably not an attribute of certificate-authority resource, right? I'm trying your implementation:
[standalone@localhost:9990 /] /subsystem=elytron/certificate-authority=Buypass:read-attribute(name=<TAB>
staging-url url
[standalone@localhost:9990 /] /subsystem=elytron/certificate-authority=Buypass:read-attribute(name=name)
{
"outcome" => "failed",
"failure-description" => "WFLYCTL0201: Unknown attribute 'name'",
"rolled-back" => true
}
[standalone@localhost:9990 /]
There was a problem hiding this comment.
@marekkopecky In your example, name has value "Buypass". When you define certificate-authority in the standalone.xml, it looks like this:
<certificate-authorities>
<certificate-authority name="Buypass" url="http://www.test.com"/>
</certificate-authorities>I think CLI in this case is searching through attributes of resource Buypass specifically, which does not contain additional name attribute.
There was a problem hiding this comment.
@Skyllarr Ok, thank you for the explanation.
…thority-account configurable
…thority-account configurable