Skip to content

Eventing TLS: Add Addresses to Addressable and additional Address fields#2713

Merged
knative-prow[bot] merged 2 commits intoknative:mainfrom
vishal-chdhry:update-addressable
Apr 3, 2023
Merged

Eventing TLS: Add Addresses to Addressable and additional Address fields#2713
knative-prow[bot] merged 2 commits intoknative:mainfrom
vishal-chdhry:update-addressable

Conversation

@vishal-chdhry
Copy link
Contributor

@vishal-chdhry vishal-chdhry commented Apr 3, 2023

Changes

Addressable and AddressStatus types the Eventing TLS feature track describes

/kind feature
/area API

Fixes #2711

Release Note

Add Addresses to Addressable.
Add Name and CACerts field to Address

Docs


Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
@knative-prow
Copy link

knative-prow bot commented Apr 3, 2023

@vishal-chdhry: The label(s) kind/<kind> cannot be applied, because the repository doesn't have them.

Details

In response to this:

Changes

Closes: #2711
Addressable and AddressStatus types the Eventing TLS feature track describes

/kind

Fixes #

Release Note


Docs


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.

@knative-prow knative-prow bot requested review from aliok and pierDipi April 3, 2023 12:47
@knative-prow
Copy link

knative-prow bot commented Apr 3, 2023

Welcome @vishal-chdhry! It looks like this is your first PR to knative/pkg 🎉

@knative-prow knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 3, 2023
@knative-prow
Copy link

knative-prow bot commented Apr 3, 2023

Hi @vishal-chdhry. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

Copy link
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

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

Thanks!

Some suggestions to make the comments clearer and more navigable.
I put my comments on the v1 API but those apply to v1beta1 as well.

Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
@vishal-chdhry
Copy link
Contributor Author

  1. Should I update the public document with these changes as well?
  2. And I manually updated the populate method for addressable, is there any other methods that I have to create/update
  3. Are there any tests that I should write for addressable_types_test.go

@pierDipi
Copy link
Member

pierDipi commented Apr 3, 2023

  1. Should I update the public document with these changes as well?

There is no need, the doc is usually abstract and high level, some details like comments can vary during the implementation.

  1. And I manually updated the populate method for addressable, is there any other methods that I have to create/update

I think CI tests will tell us if something is breaking and needs updates

/ok-to-test

  1. Are there any tests that I should write for addressable_types_test.go

No, we will add tests covering new fields as part of #2714

@knative-prow knative-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 3, 2023
Copy link
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Apr 3, 2023
@pierDipi
Copy link
Member

pierDipi commented Apr 3, 2023

Thanks @vishal-chdhry

@knative-prow
Copy link

knative-prow bot commented Apr 3, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi, Vishal-Chdhry

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 3, 2023
@knative-prow knative-prow bot merged commit b7f2774 into knative:main Apr 3, 2023
@pierDipi pierDipi changed the title Made changes to addressable and address status. Eventing TLS: Add Addresses to Addressable and additional Address fields Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eventing TLS: Update Addressable and AddressStatus

2 participants