Skip to content
This repository has been archived by the owner on Aug 14, 2019. It is now read-only.

%acme: use cert from clay files #1151

Merged
merged 18 commits into from
Apr 23, 2019
Merged

%acme: use cert from clay files #1151

merged 18 commits into from
Apr 23, 2019

Conversation

Fang-
Copy link
Member

@Fang- Fang- commented Apr 19, 2019

@joemfb as mentioned over email!

Does what it says on the can. Successfully tested on ~paldev.

@Fang- Fang- added the apps label Apr 19, 2019
@Fang- Fang- requested a review from joemfb April 19, 2019 21:51
@ohAitch
Copy link
Contributor

ohAitch commented Apr 19, 2019 via email

@joemfb
Copy link
Member

joemfb commented Apr 23, 2019

yes, we do @ohAitch.

Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

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

I don't particularly like the hardcoded paths, and feel that this isn't strictly functionality that should live in :acme. That being said, the right way to do this requires some more functionality for managing multiple domains (and the intersection between a set of domains and a certificate). Doing that right probably requires a more-complete PKCS10 parser. Barring all that, this is a fine solution.

LGTM.

@Fang-
Copy link
Member Author

Fang- commented Apr 23, 2019

Thanks @joemfb! I agree with you on both counts. I'll add a TODO for factoring it out eventually, to more strongly signal the temporary intention of this code, and then merge this in.

@Fang- Fang- changed the base branch from master to next April 23, 2019 19:38
@Fang- Fang- merged commit e770e9f into next Apr 23, 2019
@Fang- Fang- deleted the acme-local branch April 23, 2019 19:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants