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

Display Let's Encrypt certificate status #410

Merged
merged 4 commits into from
May 10, 2019

Conversation

EtienneM
Copy link
Member

@EtienneM EtienneM commented Apr 16, 2019

Fix #409

$ go build -o cli ./scalingo && ./cli -a my-app domains
+----------------+--------------------------------+
|     DOMAIN     |            TLS/SSL             |
+----------------+--------------------------------+
| example.com    | (LE In use) Valid until        |
|                | 2019-07-01 10:57:26 +0200 CEST |
| *.example.net  | (LE Error) Valid until         |
|                | 2019-04-12 19:40:07 +0200 CEST |
| *.example.eu   | (LE In use) Valid until        |
|                | 2019-06-12 09:00:11 +0200 CEST |
| example.net    |                              - |
| example.eu     |                              - |
+----------------+--------------------------------+

@EtienneM EtienneM marked this pull request as ready for review April 16, 2019 15:05
Copy link
Contributor

@johnsudaar johnsudaar left a comment

Choose a reason for hiding this comment

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

Not sure if i like the current implementation. It seems messy. Shouldn't we add a new column ?

@EtienneM
Copy link
Member Author

I don't like it either, but I'm not sure a new column is better. How do you se this new column? Naming it Let's Encrypt and only adding it if there is at least a domain with a Let's Encrypt certificate?

@johnsudaar
Copy link
Contributor

I don't like it either, but I'm not sure a new column is better. How do you se this new column? Naming it Let's Encrypt and only adding it if there is at least a domain with a Let's Encrypt certificate?

Why not yep.

@yannski
Copy link
Member

yannski commented Apr 23, 2019

Maybe the column should be named "TLS". Therefore Let's Encrypt status and Custom certificate status should be merged? BTW we have the same semantic "problem" on the web dashboard.

@EtienneM EtienneM force-pushed the fix/409/letsencrypt_error branch from fe1b11a to 700172a Compare May 2, 2019 13:57
@EtienneM
Copy link
Member Author

EtienneM commented May 2, 2019

$ go build -o cli ./scalingo && ./cli -a my-app domains
+----------------+-----------------------+
|     DOMAIN     |        TLS/SSL        |
+----------------+-----------------------+
| abcde.azert.me | Let's Encrypt: In use |
| *.azerty.net   | Let's Encrypt: Error  |
| *.azerty.eu    | Let's Encrypt: In use |
| azerty.net     |                     - |
| azerty.eu      |                     - |
+----------------+-----------------------+

@EtienneM EtienneM requested a review from johnsudaar May 2, 2019 13:59
@johnsudaar
Copy link
Contributor

And for custom certificates ?

@EtienneM
Copy link
Member Author

EtienneM commented May 2, 2019

The code didn't change for custom certificate so it displays Valid until 2019-07-01 10:57:26 +0200 CEST

@johnsudaar johnsudaar merged commit 9859c3e into master May 10, 2019
@EtienneM EtienneM deleted the fix/409/letsencrypt_error branch May 17, 2021 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[domains] Let's Encrypt status error is not correctly handled
3 participants