Skip to content

Promote strict subscriber to beta#6473

Merged
knative-prow[bot] merged 2 commits into
knative:mainfrom
pierDipi:KNATIVE-5756_Promote-strict-subscriber-to-beta
Aug 4, 2022
Merged

Promote strict subscriber to beta#6473
knative-prow[bot] merged 2 commits into
knative:mainfrom
pierDipi:KNATIVE-5756_Promote-strict-subscriber-to-beta

Conversation

@pierDipi
Copy link
Copy Markdown
Member

@pierDipi pierDipi commented Aug 4, 2022

Signed-off-by: Pierangelo Di Pilato pierdipi@redhat.com

Part of #5756

Proposed Changes

  • Promote strict subscriber to beta
  • Enable feature by default

Pre-review Checklist

  • 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

The strict subscriber feature for Subscription API is now beta and enabled by default.
When the reply field is specified without a subscriber, the reply field won't be used as a subscriber by default.

@knative-prow knative-prow Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release labels Aug 4, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 4, 2022

Codecov Report

Merging #6473 (bf0ca1d) into main (37dc195) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #6473      +/-   ##
==========================================
+ Coverage   82.15%   82.18%   +0.02%     
==========================================
  Files         235      235              
  Lines       11694    11712      +18     
==========================================
+ Hits         9607     9625      +18     
  Misses       1620     1620              
  Partials      467      467              
Impacted Files Coverage Δ
pkg/apis/messaging/v1/subscription_types.go 66.66% <ø> (ø)
pkg/broker/ingress/ingress_handler.go 77.33% <0.00%> (ø)
pkg/adapter/apiserver/events/events.go 94.11% <0.00%> (+0.30%) ⬆️
pkg/kncloudevents/message_receiver.go 93.50% <0.00%> (+1.31%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
@pierDipi pierDipi force-pushed the KNATIVE-5756_Promote-strict-subscriber-to-beta branch from d0a1b39 to 512f508 Compare August 4, 2022 10:49
@knative-prow knative-prow Bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 4, 2022
@pierDipi
Copy link
Copy Markdown
Member Author

pierDipi commented Aug 4, 2022

/test upgrade-tests_eventing_main

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
@pierDipi pierDipi force-pushed the KNATIVE-5756_Promote-strict-subscriber-to-beta branch from fb394f8 to bf0ca1d Compare August 4, 2022 13:58
@pierDipi pierDipi changed the title [WIP] Promote strict subscriber to beta Promote strict subscriber to beta Aug 4, 2022
@knative-prow knative-prow Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 4, 2022
@pierDipi
Copy link
Copy Markdown
Member Author

pierDipi commented Aug 4, 2022

/assign @lionelvillard
/cc @gab-satchi

@lionelvillard
Copy link
Copy Markdown
Contributor

I'm looking at the downstream failures (all related to go 1.18)

/approve
/lgtm

@knative-prow knative-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 4, 2022
@knative-prow
Copy link
Copy Markdown

knative-prow Bot commented Aug 4, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lionelvillard, pierDipi

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:
  • OWNERS [lionelvillard,pierDipi]

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 merged commit d99685b into knative:main Aug 4, 2022
@pierDipi pierDipi deleted the KNATIVE-5756_Promote-strict-subscriber-to-beta branch August 4, 2022 17:18
creydr added a commit to creydr/knative-eventing that referenced this pull request Nov 14, 2022
…o GA

As per knative/specs#30 the spec was changed
from requiring one of `spec.subscriber` or `spec.reply` to be set to
require that `spec.subscriber` is set.

As part of this change the `strict-subscriber` feature flag was
introduced to disable this validation and promoted to beta (enabled by
default) in 1.7 (knative#6473).

This commit removes this feature flag again so the validation can't be
disabled anymore.

Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
creydr added a commit to creydr/knative-eventing that referenced this pull request Nov 16, 2022
…o GA

As per knative/specs#30 the spec was changed
from requiring one of `spec.subscriber` or `spec.reply` to be set to
require that `spec.subscriber` is set.

As part of this change the `strict-subscriber` feature flag was
introduced to disable this validation and promoted to beta (enabled by
default) in 1.7 (knative#6473).

This commit removes this feature flag again so the validation can't be
disabled anymore.

Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
creydr added a commit to creydr/knative-eventing that referenced this pull request Nov 16, 2022
…o GA

As per knative/specs#30 the spec was changed
from requiring one of `spec.subscriber` or `spec.reply` to be set to
require that `spec.subscriber` is set.

As part of this change the `strict-subscriber` feature flag was
introduced to disable this validation and promoted to beta (enabled by
default) in 1.7 (knative#6473).

This commit removes this feature flag again so the validation can't be
disabled anymore.

Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
knative-prow Bot pushed a commit that referenced this pull request Nov 16, 2022
Fixes #5756

As per knative/specs#30 the spec was changed
from requiring one of `spec.subscriber` or `spec.reply` to be set to
require that `spec.subscriber` is set.

As part of this change the `strict-subscriber` feature flag was
introduced to disable this validation (#5762) and promoted to beta
(enabled by default) in 1.7 (#6473).

:broom: This commit removes this feature flag again so the validation
can't be disabled anymore.

## Proposed Changes
* Remove `strict-subscriber` feature flag
* Promote strict subscriber validation to GA

### Pre-review Checklist

- [ ] **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**
```release-note
Remove the possibility to disable strict subscriber validation.
When the reply field is specified without a subscriber, the reply field won't be used as a subscriber by default and the subscriber validation will fail.
```

Signed-off-by: Christoph Stäbler <cstabler@redhat.com>
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. area/test-and-release Test infrastructure, tests or release lgtm Indicates that a PR is ready to be merged. 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.

2 participants