-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add a meaningful User-Agent. #422
Conversation
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 good to me! I'll cherry pick this into 0.2 as well once merged. Thanks for putting this together.
pkg/issuer/acme/acme.go
Outdated
ua string | ||
} | ||
|
||
const Version = "v0.2.3" |
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.
We can use pkg/util.AppVersion
here instead to save it being maintained in multiple places
/ok-to-test |
@jsha I've updated your PR with an additional commit to fix test failures, as well as using Thanks for this PR - I'll get this cherry picked into release 0.2 once merged, and this will be the final PR to go into 0.2 😄. #309 will also switch us to use acmev2, and will also encompass this change 😄. I'll be cutting 0.3-alpha.1 later today hopefully, and I'd imagine a number of users should be willing to test it out on account of wildcard cert support, so we should get some data back 😄 |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: munnerz 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 |
/retest |
1 similar comment
/retest |
Seems we're hitting a test flake when boulder is installed. It appears to be happening everywhere now too 🤔 I'll investigate further. I think it's due to a lack of resources on our build cluster. |
/retest |
/retest |
1 similar comment
/retest |
/retest |
…stream-release-0.2 Automated cherry pick of #422
…uests This code existed in cert-manager once before and I'm reviving it. Here's the history: * Added: cert-manager#422 * Moved: cert-manager#432 * Obsoleted: cert-manager#797 * Deleted: cert-manager#966 Signed-off-by: Richard Wall <richard.wall@venafi.com>
Related to #407.