Ce overrides validation

[gabo1208]

Fixes #5638

Release Note

Validate CloudEvent overrides as defined by the CloudEvent spec.

[codecov]

Codecov Report

Merging #5730 (c9e3156) into main (cf630ed) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5730      +/-   ##
==========================================
- Coverage   82.68%   82.65%   -0.03%     
==========================================
  Files         200      202       +2     
  Lines        6261     6326      +65     
==========================================
+ Hits         5177     5229      +52     
- Misses        750      758       +8     
- Partials      334      339       +5     

Impacted Files	Coverage Δ
pkg/apis/sources/v1/apiserver_validation.go	100.00% <100.00%> (ø)
pkg/apis/sources/v1/container_validation.go	83.33% <100.00%> (+0.98%)	⬆️
pkg/apis/sources/v1/ping_validation.go	100.00% <100.00%> (ø)
pkg/apis/sources/v1/sinkbinding_validation.go	100.00% <100.00%> (ø)
pkg/apis/sources/v1beta2/ping_validation.go	100.00% <100.00%> (ø)
pkg/channel/attributes/knative_error.go	100.00% <0.00%> (ø)
pkg/resolver/addressable_resolver.go	100.00% <0.00%> (ø)
pkg/resolver/mapping_resolver.go	72.91% <0.00%> (ø)
pkg/channel/message_dispatcher.go	79.03% <0.00%> (+0.51%)	⬆️
pkg/reconciler/pingsource/controller.go	87.87% <0.00%> (+2.16%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf630ed...c9e3156. Read the comment docs.

[pierDipi]

Can we do this for all other sources as well?

[gabo1208]

Sure, but, I don't know exactly where that should be. If you can give me a hand with this :)

[pierDipi]

Sure, we need to go through every file in pkg/apis/sources/<version>/*_validation.go, and check if the embedded SourceSpec is validated, if not then we add the same check that you added for the PingSource (line 97).

For example, for ApiServerSource, we don't validate SourceSpec [1], so we need to add the check errs = errs.Also(cs.SourceSpec.Validate(ctx)) here

eventing/pkg/apis/sources/v1/apiserver_validation.go

Line 75 in 407f3cb

Also, if we can add simple unit tests for each source to prevent regression that would be great too.

Hope this helps, thanks!

[1]

eventing/pkg/apis/sources/v1/apiserver_validation.go

Lines 39 to 77 in 407f3cb

	func (cs *ApiServerSourceSpec) Validate(ctx context.Context) *apis.FieldError {
	var errs *apis.FieldError
	// Validate mode, if can be empty or set as certain value
	switch cs.EventMode {
	case ReferenceMode, ResourceMode:
	// EventMode is valid.
	default:
	errs = errs.Also(apis.ErrInvalidValue(cs.EventMode, "mode"))
	}
	// Validate sink
	errs = errs.Also(cs.Sink.Validate(ctx).ViaField("sink"))
	if len(cs.Resources) == 0 {
	errs = errs.Also(apis.ErrMissingField("resources"))
	}
	for i, res := range cs.Resources {
	_, err := schema.ParseGroupVersion(res.APIVersion)
	if err != nil {
	errs = errs.Also(apis.ErrInvalidValue(res.APIVersion, "apiVersion").ViaFieldIndex("resources", i))
	}
	if strings.TrimSpace(res.Kind) == "" {
	errs = errs.Also(apis.ErrMissingField("kind").ViaFieldIndex("resources", i))
	}
	}
	if cs.ResourceOwner != nil {
	_, err := schema.ParseGroupVersion(cs.ResourceOwner.APIVersion)
	if err != nil {
	errs = errs.Also(apis.ErrInvalidValue(cs.ResourceOwner.APIVersion, "apiVersion").ViaField("owner"))
	}
	if strings.TrimSpace(cs.ResourceOwner.Kind) == "" {
	errs = errs.Also(apis.ErrMissingField("kind").ViaField("owner"))
	}
	}
	return errs
	}

[pierDipi]

Please remove this.

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/sources/v1/container_validation.go	88.2%	88.9%	0.7

[gabo1208]

/retest

[pierDipi]

Thanks!

Can you add a release note in the PR body?
There is an example in the PR template in the .github folder.

/lgtm
/approve
/kind bug

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

• OWNERS [pierDipi]

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