A name and selector MUST NOT be defined together #1005

baijum · 2021-08-04T14:42:51Z

A name and selector MUST NOT be defined in the same application
reference

codecov · 2021-08-04T16:21:33Z

Codecov Report

Merging #1005 (2a4617b) into master (bc69c60) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 2a4617b differs from pull request most recent head ca15119. Consider uploading reports for the commit ca15119 to get more accurate results

@@            Coverage Diff             @@
##           master    #1005      +/-   ##
==========================================
+ Coverage   58.03%   58.08%   +0.04%     
==========================================
  Files          29       29              
  Lines        1642     1646       +4     
==========================================
+ Hits          953      956       +3     
  Misses        563      563              
- Partials      126      127       +1

Impacted Files	Coverage Δ
apis/binding/v1alpha1/servicebinding_webhook.go	`63.15% <100.00%> (+21.49%)`	⬆️
apis/spec/v1alpha3/servicebinding_webhook.go	`63.15% <100.00%> (+21.49%)`	⬆️
...kg/reconcile/pipeline/context/spec_binding_impl.go	`72.54% <100.00%> (ø)`
pkg/binding/model.go	`87.50% <0.00%> (-2.70%)`	⬇️
pkg/binding/definition.go	`47.54% <0.00%> (-2.46%)`	⬇️
pkg/binding/annotationmapper.go	`87.34% <0.00%> (-0.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 bc69c60...ca15119. Read the comment docs.

pedjak

we should not even allow creating service binding instances that have name and selector defined.

Validation webhooks are our friend here - the check should be implemented there instead.

baijum · 2021-09-11T09:53:17Z

we should not even allow creating service binding instances that have name and selector defined.

Validation webhooks are our friend here - the check should be implemented there instead.

Done.

pedjak

the checks must exists for both APIs and acceptance tests should verify if they are active.

apis/binding/v1alpha1/servicebinding_webhook.go

apis/spec/v1alpha2/servicebinding_types.go

test/acceptance/features/bindAppToService.feature

apis/binding/v1alpha1/servicebinding_webhook.go

apis/spec/v1alpha3/servicebinding_webhook.go

pkg/reconcile/pipeline/context/spec_binding_impl.go

pedjak · 2021-10-06T05:33:45Z

pkg/reconcile/pipeline/context/spec_binding_impl_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/base64"
 	e "errors"
+


nit: whitespace change, please revert.

test/acceptance/features/bindAppToService.feature

pedjak · 2021-10-06T05:36:16Z

test/acceptance/features/bindAppToService.feature

+            """
+
+    @negative
+    Scenario: Cannot create Service Binding with name and label selector in the spec API


annotation this scenario with @spec tag.

test/acceptance/features/bindAppToService.feature

pedjak

/lgtm

please squash commits

A name and selector MUST NOT be defined in the same application reference Use webhook to validate Fix redhat-developer#974 Signed-off-by: Baiju Muthukadan <[email protected]>

baijum · 2021-10-06T15:17:33Z

please squash commits

Done

pedjak · 2021-10-06T16:13:28Z

/lgtm
/approve

openshift-ci · 2021-10-06T16:13:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pedjak

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~~ [pedjak]

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

baijum · 2021-10-06T18:33:43Z

/test 4.7-acceptance

openshift-ci bot requested review from pmacik and pratikjagrut August 4, 2021 14:42

baijum force-pushed the name-selector branch from 2e9f52e to 69798d2 Compare August 4, 2021 14:58

baijum requested a review from pedjak August 31, 2021 11:43

pedjak suggested changes Sep 1, 2021

View reviewed changes

openshift-ci bot added the needs-rebase label Sep 9, 2021

baijum force-pushed the name-selector branch from 69798d2 to 7513c72 Compare September 10, 2021 15:30

openshift-ci bot removed the needs-rebase label Sep 10, 2021

baijum force-pushed the name-selector branch from b36ee8d to bf63f79 Compare September 11, 2021 09:55

pedjak removed the request for review from pratikjagrut September 12, 2021 09:09

pedjak suggested changes Sep 13, 2021

View reviewed changes

apis/binding/v1alpha1/servicebinding_webhook.go Outdated Show resolved Hide resolved

baijum force-pushed the name-selector branch 3 times, most recently from 8bd5b5c to 4e61c4a Compare September 14, 2021 11:06

pedjak reviewed Sep 15, 2021

View reviewed changes

apis/spec/v1alpha2/servicebinding_types.go Outdated Show resolved Hide resolved

baijum force-pushed the name-selector branch 3 times, most recently from 752e2e9 to 2d52808 Compare September 21, 2021 11:40

baijum commented Sep 21, 2021

View reviewed changes

test/acceptance/features/bindAppToService.feature Outdated Show resolved Hide resolved

baijum commented Sep 21, 2021

View reviewed changes

test/acceptance/features/bindAppToService.feature Outdated Show resolved Hide resolved

baijum force-pushed the name-selector branch from 7bf7f38 to 573f8b5 Compare September 22, 2021 08:47

openshift-ci bot added the needs-rebase label Oct 3, 2021

baijum force-pushed the name-selector branch from 573f8b5 to 875331d Compare October 4, 2021 07:29

openshift-ci bot removed the needs-rebase label Oct 4, 2021

baijum force-pushed the name-selector branch 2 times, most recently from f17848c to 3bf4079 Compare October 5, 2021 06:08

pedjak suggested changes Oct 6, 2021

View reviewed changes

baijum force-pushed the name-selector branch from 2a4617b to 89adbd2 Compare October 6, 2021 09:55

pedjak suggested changes Oct 6, 2021

View reviewed changes

openshift-ci bot assigned pedjak Oct 6, 2021

openshift-ci bot added the lgtm label Oct 6, 2021

A name and selector MUST NOT be defined together

6ad0ddd

A name and selector MUST NOT be defined in the same application reference Use webhook to validate Fix redhat-developer#974 Signed-off-by: Baiju Muthukadan <[email protected]>

baijum force-pushed the name-selector branch from ca15119 to 6ad0ddd Compare October 6, 2021 12:13

openshift-ci bot removed the lgtm label Oct 6, 2021

pedjak approved these changes Oct 6, 2021

View reviewed changes

openshift-ci bot added the lgtm label Oct 6, 2021

openshift-ci bot added the approved label Oct 6, 2021

openshift-merge-robot merged commit 254625f into redhat-developer:master Oct 6, 2021

baijum deleted the name-selector branch October 7, 2021 06:42

A name and selector MUST NOT be defined together #1005

A name and selector MUST NOT be defined together #1005

Uh oh!

Conversation

baijum commented Aug 4, 2021

Uh oh!

codecov bot commented Aug 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pedjak left a comment

Choose a reason for hiding this comment

Uh oh!

baijum commented Sep 11, 2021

Uh oh!

pedjak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pedjak Oct 6, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pedjak Oct 6, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pedjak left a comment

Choose a reason for hiding this comment

Uh oh!

baijum commented Oct 6, 2021

Uh oh!

pedjak commented Oct 6, 2021

Uh oh!

openshift-ci bot commented Oct 6, 2021

Uh oh!

baijum commented Oct 6, 2021

Uh oh!

Uh oh!

codecov bot commented Aug 4, 2021 •

edited

Loading