Create Cert-Manager resources#6849
Conversation
|
Hi @Rahul-Kumar-prog. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #6849 +/- ##
==========================================
- Coverage 80.45% 79.96% -0.49%
==========================================
Files 236 237 +1
Lines 12213 12354 +141
==========================================
+ Hits 9826 9879 +53
- Misses 1896 1981 +85
- Partials 491 494 +3 see 8 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
@Rahul-Kumar-prog: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There was a problem hiding this comment.
There are 3 issuers but we only need 1 issuer and indeed the content of the 3 is the same, so we can remove the 3 different issuers and only have 1, called selfsigned-issuer.yaml.
In addition, the original issue describes this:
The Certificate resources and the Issuer should then be bundled in a eventing-tls-networking.yaml artifact during the release
so we need to:
- generate such artifact in https://github.com/knative/eventing/blob/main/hack/generate-yamls.sh
- apply it as part of the install_cert_manager function
- add the artifact to the released YAMLs:
eventing/hack/generate-yamls.sh
Line 58 in c59bd1f
|
@pierDipi i will do this in some time. Last Saturday my mom expired I need sometime please. |
|
Sorry to hear that Rahul, take whatever time you need |
| apiVersion: cert-manager.io/v1 | ||
| kind: Issuer | ||
| metadata: | ||
| name: selfsigned-issuer | ||
| namespace: knative-eventing | ||
| spec: | ||
| selfSigned: {} | ||
|
No newline at end of file |
There was a problem hiding this comment.
aren't these all the same?
I'd potentially also change the name, to be a bit more explicit
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
|
/ok-to-test I updated the PR with some minor changes to unblock iteration 2 Eventing TLS (view) |
|
@matzew @gab-satchi @vishal-chdhry would you mind reviewing this PR? |
|
@pierDipi: GitHub didn't allow me to request PR reviews from the following users: Vishal-Chdhry. Note that only knative members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gab-satchi, pierDipi, Rahul-Kumar-prog The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes knative#6837 <!-- Please include the 'why' behind your changes if no issue exists --> ## Proposed Changes <!-- Please categorize your changes: - 🎁 Add new feature - 🐛 Fix bug - 🧹 Update or clean up current behavior - 🗑️ Remove feature or internal logic --> - - - ### Pre-review Checklist <!-- If these boxes are not checked, you will be asked to complete these requirements or explain why they do not apply to your PR. --> - [ ] **At least 80% unit test coverage** - [ ] **E2E tests** for any new behavior - [ ] **Docs PR** for any user-facing impact - [ ] **Spec PR** for any new API feature - [ ] **Conformance test** for any change to the spec **Release Note** <!-- 📄 If this change has user-visible impact, write a release note in the block below. Include the string "action required" if additional action is required of users switching to the new release, for example in case of a breaking change. Write as if you are speaking to users, not other Knative contributors. If this change has no user-visible impact, no release note is needed. --> ```release-note ``` **Docs** <!-- 📖 If this change has user-visible impact, link to an issue or PR in https://github.com/knative/docs. --> **Images of cert-manager resource (Certificates & Issures test)  --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierdipi@redhat.com>
Fixes #6837
Proposed Changes
Pre-review Checklist
Release Note
Docs
**Images of cert-manager resource (Certificates & Issures test)
