-
Notifications
You must be signed in to change notification settings - Fork 4
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
chore(updatecli): add an updatecli manifest to automatically create a PR to renew certificates for trusted.ci and cert.ci #191
chore(updatecli): add an updatecli manifest to automatically create a PR to renew certificates for trusted.ci and cert.ci #191
Conversation
…iration date for trusted.ci and cert.ci
and
|
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.
Good job,n almost there!
- A few blockers:
- shellcheck errors about shebang/options
- Title isn't clear about "what" is updated with its expiry date
- Nitpicks:
- Can you add a comment (eventually a block comment) to explain why using
file
kind instead of terraform? - To be thought: the PR which will be opened by this updatecli manfiest does not point to any procedure about "where" to update the new password. worth checking if we can custiomize the PR body with updatecli
- Can you add a comment (eventually a block comment) to explain why using
Co-authored-by: Damien Duportal <damien.duportal@gmail.com>
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.
There are errors related to date
, not sure to understand them at first sight
the date with as for now, we use a more conventional date working alike gnudate: and |
scmid: default | ||
spec: | ||
title: Generate new letsencrypt certificate for cert.ci new expiration date {{ source "nextExpiry" }} | ||
description: paste this certicate output here https://github.com/jenkins-infra/jenkins-infra/blob/587d065d9fc609db2cd5ececc01a8159657a73fa/hieradata/clients/controller.cert.ci.jenkins.io.yaml#L30 |
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.
this should provide information within the body of the PullRequest
scmid: default | ||
spec: | ||
title: Generate new letsencrypt certificate for trusted.ci new expiration date {{ source "nextExpiry" }} | ||
description: paste this certicate output here https://github.com/jenkins-infra/jenkins-infra/blob/587d065d9fc609db2cd5ececc01a8159657a73fa/hieradata/clients/controller.trusted.ci.jenkins.io.yaml#L35 |
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.
this should provide information within the body of the PullRequest
scmid: default | ||
spec: | ||
title: Generate new letsencrypt certificate for trusted.ci new expiration date {{ source "nextExpiry" }} | ||
description: paste this certicate output here https://github.com/jenkins-infra/jenkins-infra/blob/587d065d9fc609db2cd5ececc01a8159657a73fa/hieradata/clients/controller.trusted.ci.jenkins.io.yaml#L35 |
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.
paste this certicate output
Which output more precisely?
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.
more precise now
@@ -56,6 +56,6 @@ actions: | |||
kind: github/pullrequest | |||
scmid: default | |||
spec: | |||
title: Generate a new password for the Azure Service Principal "cert.ci.jenkins.io - Let's Encrypt" with (https://github.com/jenkins-infra/jenkins-infra/blob/d76e72a6c1ee06a0d321ed4d04c5d3e1b81d061d/hieradata/clients/controller.cert.ci.jenkins.io.yaml#L27) | |||
title: Generate a new password for the Azure Service Principal "cert.ci.jenkins.io - Let's Encrypt". Set the new password value in the following encrypted file: <https://github.com/jenkins-infra/jenkins-infra/blob/d76e72a6c1ee06a0d321ed4d04c5d3e1b81d061d/hieradata/clients/controller.cert.ci.jenkins.io.yaml#L27> (open a PR : https://github.com/jenkins-infra/jenkins-infra/edit/production/hieradata/clients/controller.cert.ci.jenkins.io.yaml). |
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.
title: Generate a new password for the Azure Service Principal "cert.ci.jenkins.io - Let's Encrypt". Set the new password value in the following encrypted file: <https://github.com/jenkins-infra/jenkins-infra/blob/d76e72a6c1ee06a0d321ed4d04c5d3e1b81d061d/hieradata/clients/controller.cert.ci.jenkins.io.yaml#L27> (open a PR : https://github.com/jenkins-infra/jenkins-infra/edit/production/hieradata/clients/controller.cert.ci.jenkins.io.yaml). | |
title: Generate a new password for the Azure Service Principal "cert.ci.jenkins.io - Let's Encrypt" | |
description: Set the new password value at `sp_client_secret` in the `profile::letsencrypt::dns_azure:` section of the encrypted file: https://github.com/jenkins-infra/jenkins-infra/blob/production/hieradata/clients/controller.cert.ci.jenkins.io.yaml. |
Add description, no "absolute" ref, and removing the "open a PR link", a bit redundant.
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.
[2024-03-22T10:59:50.205Z] CONDITIONS:
[2024-03-22T10:59:50.205Z] ===========
[2024-03-22T10:59:50.205Z]
[2024-03-22T10:59:50.205Z] checkIfExpirySoonExpiredcert
[2024-03-22T10:59:50.205Z] ----------------------------
[2024-03-22T10:59:50.205Z] The shell 🐚 command "/bin/sh /tmp/updatecli/bin/d8a07761ca8468705bdcd187510d8a87a1813a2d546f279433b40952ebee3875.sh" exited on error (exit code 1) with the following output:
[2024-03-22T10:59:50.205Z] ----
[2024-03-22T10:59:50.205Z] not yet expired
[2024-03-22T10:59:50.205Z] ----
[2024-03-22T10:59:50.205Z]
[2024-03-22T10:59:50.205Z] command stderr output was:
[2024-03-22T10:59:50.205Z] ----
[2024-03-22T10:59:50.205Z] + currentexpirydate=2024-04-03T21:00:00Z
[2024-03-22T10:59:50.205Z] + DATE_BIN=date
[2024-03-22T10:59:50.205Z] + command -v gdate
[2024-03-22T10:59:50.205Z] + command -v date
[2024-03-22T10:59:50.205Z] ++ date --utc +%s
[2024-03-22T10:59:50.205Z] + currentdateepoch=1711105190
[2024-03-22T10:59:50.205Z] ++ date +%s -d 2024-04-03T21:00:00Z
[2024-03-22T10:59:50.205Z] + expirydateepoch=1712178000
[2024-03-22T10:59:50.205Z] + datediff=12
[2024-03-22T10:59:50.205Z] + '[' 12 -lt 10 ']'
[2024-03-22T10:59:50.205Z] + echo 'not yet expired'
[2024-03-22T10:59:50.205Z] + exit 1
[2024-03-22T10:59:50.205Z]
[2024-03-22T10:59:50.205Z] ----
[2024-03-22T10:59:50.205Z] shell command failed. Expected exit code 0 but got 1
[2024-03-22T10:59:50.205Z] ✗ shell condition of type "console/output" not passing
[2024-03-22T10:59:50.205Z]
[2024-03-22T10:59:50.205Z]
[2024-03-22T10:59:50.205Z] TARGETS
[2024-03-22T10:59:50.205Z] ========
[2024-03-22T10:59:50.205Z]
[2024-03-22T10:59:50.205Z] updateNextExpirycert
[2024-03-22T10:59:50.205Z] --------------------
[2024-03-22T10:59:50.205Z] Conditions [checkIfExpirySoonExpiredcert] not met. Skipping execution of the target["updateNextExpirycert"]
=> that looks good!
- I would set the diff date to 3 weeks (e.g. 21 days) to map to our calendars. It would let us enough time to "detect, understand and plan" the change: 10 days looks really short. WDYT? (besides, that change would trigger the new PR so a good way to fully validate)
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.
LGTM!
The output checks show that the 2 dates will have to be updated!
[2024-04-05T12:26:51.670Z] TARGETS
[2024-04-05T12:26:51.670Z] ========
[2024-04-05T12:26:51.670Z]
[2024-04-05T12:26:51.670Z] updateNextExpirycert
[2024-04-05T12:26:51.670Z] --------------------
[2024-04-05T12:26:51.670Z]
[2024-04-05T12:26:51.670Z] **Dry Run enabled**
[2024-04-05T12:26:51.670Z]
[2024-04-05T12:26:51.670Z] "locals.tf" updated with [dry run] content "${1}2024-07-04T00:00:00Z\""
[2024-04-05T12:26:51.670Z]
[2024-04-05T12:26:51.670Z] ```
[2024-04-05T12:26:51.670Z] --- locals.tf
[2024-04-05T12:26:51.670Z] +++ locals.tf
[2024-04-05T12:26:51.670Z] @@ -15,7 +15,7 @@
[2024-04-05T12:26:51.670Z]
[2024-04-05T12:26:51.670Z] lets_encrypt_dns_challenged_domains = {
[2024-04-05T12:26:51.670Z] "trusted.ci.jenkins.io" = "2024-04-03T20:00:00Z"
[2024-04-05T12:26:51.670Z] - "cert.ci.jenkins.io" = "2024-04-03T21:00:00Z"
[2024-04-05T12:26:51.670Z] + "cert.ci.jenkins.io" = "2024-07-04T00:00:00Z"
[2024-04-05T12:26:51.670Z] # TODO: add support for workload identities by providing an empty expiration date
[2024-04-05T12:26:51.670Z] # "<something>.jenkins.io" = ""
[2024-04-05T12:26:51.670Z] }
[2024-04-05T12:26:51.670Z]
[2024-04-05T12:26:51.670Z] ```
[2024-04-05T12:26:51.670Z]
[2024-04-05T12:26:51.670Z] ⚠ - 1 file(s) updated with "${1}2024-07-04T00:00:00Z\"":
[2024-04-05T12:26:51.670Z] * locals.tf
Great work!
provide an automatic way to update expiration date for renewing certificates, this will spawn PR to generate the new certificates