Skip to content
This repository has been archived by the owner on Jun 26, 2024. It is now read-only.

Commit

Permalink
Fix crash when annotation's sourceValue is invalid (#967) (#1015)
Browse files Browse the repository at this point in the history
A service can have a sliceOfMaps-based element annotation.  For example:

    service.binding/java-maven_port: path={.spec.ports},elementType=sliceOfMaps,sourceKey=name,sourceValue=asdf

When sourceValue is incorrect, the operator would panic, since it didn't
verify that the sourceValue was incorrect.  Now, it sets an error
condition in the reconciler (which propogates to the binding status).

Signed-off-by: Andy Sadler <ansadler@redhat.com>

Co-authored-by: Igor Sutton <isuttonl@redhat.com>
  • Loading branch information
sadlerap and Igor Sutton authored Sep 10, 2021
1 parent 2785337 commit 252987b
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 1 deletion.
15 changes: 14 additions & 1 deletion pkg/reconcile/pipeline/handler/collect/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@ import (
)

var DataNotMap = errors.New("Returned data are not a map, skip collecting")
var ErrorValueNotFound = errors.New("Value not found in map")

const (
ErrorReadingServicesReason = "ErrorReadingServices"
ErrorReadingCRD = "ErrorReadingCRD"
ErrorReadingDescriptorReason = "ErrorReadingDescriptor"
ErrorReadingBindingReason = "ErrorReadingBinding"
ErrorReadingSecret = "ErrorReadingSecret"

ValueNotFound = "ValueNotFound"
)

func PreFlight(ctx pipeline.Context) {
Expand Down Expand Up @@ -249,7 +252,17 @@ func collectItems(prefix string, ctx pipeline.Context, service pipeline.Service,
p = ""
}
for _, n := range v.MapKeys() {
collectItems(p, ctx, service, n, v.MapIndex(n).Interface())
if mapVal := v.MapIndex(n).Interface(); mapVal != nil {
collectItems(p, ctx, service, n, mapVal)
} else {
condition := apis.Conditions().NotCollectionReady().
Msg(fmt.Sprintf("Value for key %v_%v not found", prefix+k.String(), n.String())).
Reason(ValueNotFound).Build()
ctx.SetCondition(condition)
ctx.Error(ErrorValueNotFound)
ctx.StopProcessing()
return
}
}
case reflect.Slice:
for i := 0; i < v.Len(); i++ {
Expand Down
32 changes: 32 additions & 0 deletions pkg/reconcile/pipeline/handler/collect/impl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,38 @@ var _ = Describe("Collect Binding Data", func() {
shouldRetry(pipeline.HandlerFunc(collect.BindingItems), collect.ErrorReadingBindingReason, err)
})

Context("on nil value in collected bindings", func() {
It("should set an error condition and stop processing the pipeline",
func() {
serviceResource := &unstructured.Unstructured{}

ctx := mocks.NewMockContext(mockCtrl)
service := mocks.NewMockService(mockCtrl)
definition := bindingmocks.NewMockDefinition(mockCtrl)
value := bindingmocks.NewMockValue(mockCtrl)

ctx.EXPECT().Services().Return([]pipeline.Service{service}, nil)

bindingDefs := []binding.Definition{definition}
service.EXPECT().BindingDefs().Return(bindingDefs)
service.EXPECT().Resource().Return(serviceResource)

definition.EXPECT().Apply(serviceResource).Return(value, nil)
value.EXPECT().Get().Return(map[string]map[string]interface{}{"java-maven_port": {"foo": nil}})

ctx.EXPECT().SetCondition(
apis.Conditions().NotCollectionReady().
Reason(collect.ValueNotFound).
Msg("Value for key java-maven_port_foo not found").
Build())
ctx.EXPECT().Error(collect.ErrorValueNotFound)
ctx.EXPECT().StopProcessing()

collect.BindingItems(ctx)
},
)
})

Context("on returning unexpected data", func() {
var (
service *mocks.MockService
Expand Down
42 changes: 42 additions & 0 deletions test/acceptance/features/bindAppToServiceAnnotations.feature
Original file line number Diff line number Diff line change
Expand Up @@ -542,3 +542,45 @@ Feature: Bind an application to a service using annotations
"""
Then Service Binding "binding-app-via-gvk" is ready
And The application env var "BACKEND_HOST_INTERNAL_DB" has value "internal.db.stable.example.com"

Scenario: Application cannot be bound to service containing annotation with an invalid sourceValue value
Given Generic test application is running
And CustomResourceDefinition backends.stable.example.com is available
And The Custom Resource is present
"""
apiVersion: stable.example.com/v1
kind: Backend
metadata:
name: $scenario_id
annotations:
service.binding/webarrows: path={.spec.connections},elementType=sliceOfMaps,sourceKey=type,sourceValue=asdf
spec:
connections:
- type: primary
url: primary.example.com
status:
somestatus: good
"""
When Service Binding is applied
"""
apiVersion: binding.operators.coreos.com/v1alpha1
kind: ServiceBinding
metadata:
name: $scenario_id
spec:
bindAsFiles: false
services:
- group: stable.example.com
version: v1
kind: Backend
name: $scenario_id
application:
name: $scenario_id
group: apps
version: v1
resource: deployments
"""
Then Service Binding CollectionReady.status is "False"
And Service Binding CollectionReady.reason is "ValueNotFound"
And Service Binding CollectionReady.message is "Value for key webarrows_primary not found"

9 changes: 9 additions & 0 deletions test/acceptance/resources/backend_crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ spec:
type: string
environment:
type: string
connections:
type: array
items:
type: object
properties:
type:
type: string
url:
type: string
status:
type: object
properties:
Expand Down

0 comments on commit 252987b

Please sign in to comment.