diff --git a/docs/design/field-references.md b/docs/design/field-references.md index 4373517..4279527 100644 --- a/docs/design/field-references.md +++ b/docs/design/field-references.md @@ -30,7 +30,7 @@ spec: spec: sleepFor: 3 wakeupMessage: Hello, Infravators! - + --- comes later status: message: "Hello, Infravators!" @@ -57,3 +57,65 @@ spec: name: sleeper3 spec: "{{{sleeper2#spec}}}" ``` + +## Referring to ServiceBinding outputs + +When Service Catalog processes a ServiceBinding, the output is placed in a Secret +(since they might be secret). If they're not secret, it's convenient to directly +reference them in the bundle. This can be done by using `dependency:bindsecret#Data.secretkey`. +At the moment, `bindsecret` is the only parameterisation of the dependency that is allowed. + +For example: + +```yaml +apiVersion: smith.atlassian.com/v1 +kind: Bundle +metadata: + name: ab +spec: + resources: + + - name: a + spec: + object: + metadata: + name: ups-instance + apiVersion: servicecatalog.k8s.io/v1beta1 + kind: ServiceInstance + spec: + clusterServiceClassExternalName: user-provided-service + clusterServicePlanExternalName: default + parameters: + credentials: + password: mypassword + + - name: a-binding + dependsOn: + - a + spec: + object: + metadata: + name: a-binding + apiVersion: servicecatalog.k8s.io/v1beta1 + kind: ServiceBinding + spec: + instanceRef: + name: "{{a#metadata.name}}" + secretName: a-binding-secret + + - name: b + dependsOn: + - a-binding + spec: + object: + metadata: + name: ups-instance-2 + apiVersion: servicecatalog.k8s.io/v1beta1 + kind: ServiceInstance + spec: + clusterServiceClassExternalName: user-provided-service + clusterServicePlanExternalName: default + parameters: + x: y + password: "{{a-binding:bindsecret#password}}" +``` diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 19c1afe..ac39ce1 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -96,6 +96,9 @@ const ( resSi1 = "resSi1" si1 = "si1" si1uid = "si1-uid" + resSi2 = "resSi2" + si2 = "si2" + si2uid = "si2-uid" s1 = "s1" s1uid = "s1-uid" @@ -802,6 +805,147 @@ func TestController(t *testing.T) { assert.Equal(t, "specification of the created/updated object does not match the desired spec", resCond.Message) }, }, + "binding secret references are resolved": &testCase{ + mainClientObjects: []runtime.Object{ + &core_v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: s1, + Namespace: testNamespace, + UID: s1uid, + OwnerReferences: []meta_v1.OwnerReference{ + { + APIVersion: sc_v1b1.SchemeGroupVersion.String(), + Kind: "ServiceBinding", + Name: sb1, + UID: sb1uid, + Controller: &tr, + BlockOwnerDeletion: &tr, + }, + }, + }, + Data: map[string][]byte{ + "mysecret": []byte("bla"), + }, + Type: core_v1.SecretTypeOpaque, + }, + }, + scClientObjects: []runtime.Object{ + serviceInstance(true, false, false), + serviceBinding(true, false, false), + &sc_v1b1.ServiceInstance{ + TypeMeta: meta_v1.TypeMeta{ + Kind: "ServiceInstance", + APIVersion: sc_v1b1.SchemeGroupVersion.String(), + }, + ObjectMeta: meta_v1.ObjectMeta{ + Name: si2, + Namespace: testNamespace, + UID: si2uid, + Labels: map[string]string{ + smith.BundleNameLabel: bundle1, + }, + Annotations: map[string]string{ + "Secret": "bla", + }, + OwnerReferences: []meta_v1.OwnerReference{ + { + APIVersion: smith_v1.BundleResourceGroupVersion, + Kind: smith_v1.BundleResourceKind, + Name: bundle1, + UID: bundle1uid, + Controller: &tr, + BlockOwnerDeletion: &tr, + }, + { + APIVersion: sc_v1b1.SchemeGroupVersion.String(), + Kind: "ServiceBinding", + Name: sb1, + UID: sb1uid, + BlockOwnerDeletion: &tr, + }, + }, + }, + Status: sc_v1b1.ServiceInstanceStatus{ + Conditions: []sc_v1b1.ServiceInstanceCondition{ + { + Type: sc_v1b1.ServiceInstanceConditionReady, + Status: sc_v1b1.ConditionTrue, + }, + }, + }, + }, + }, + bundle: &smith_v1.Bundle{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: bundle1, + Namespace: testNamespace, + UID: bundle1uid, + }, + Spec: smith_v1.BundleSpec{ + Resources: []smith_v1.Resource{ + { + Name: resSi1, + Spec: smith_v1.ResourceSpec{ + Object: &sc_v1b1.ServiceInstance{ + TypeMeta: meta_v1.TypeMeta{ + Kind: "ServiceInstance", + APIVersion: sc_v1b1.SchemeGroupVersion.String(), + }, + ObjectMeta: meta_v1.ObjectMeta{ + Name: si1, + }, + }, + }, + }, + { + Name: resSb1, + DependsOn: []smith_v1.ResourceName{ + resSi1, + }, + Spec: smith_v1.ResourceSpec{ + Object: &sc_v1b1.ServiceBinding{ + TypeMeta: meta_v1.TypeMeta{ + Kind: "ServiceBinding", + APIVersion: sc_v1b1.SchemeGroupVersion.String(), + }, + ObjectMeta: meta_v1.ObjectMeta{ + Name: sb1, + }, + Spec: sc_v1b1.ServiceBindingSpec{ + ServiceInstanceRef: sc_v1b1.LocalObjectReference{ + Name: si1, + }, + SecretName: s1, + }, + }, + }, + }, + { + Name: resSi2, + DependsOn: []smith_v1.ResourceName{ + resSb1, + }, + Spec: smith_v1.ResourceSpec{ + Object: &sc_v1b1.ServiceInstance{ + TypeMeta: meta_v1.TypeMeta{ + Kind: "ServiceInstance", + APIVersion: sc_v1b1.SchemeGroupVersion.String(), + }, + ObjectMeta: meta_v1.ObjectMeta{ + Name: si2, + Annotations: map[string]string{ + "Secret": "{{" + resSb1 + ":bindsecret#Data.mysecret}}", + }, + }, + }, + }, + }, + }, + }, + }, + namespace: testNamespace, + enableServiceCatalog: true, + }, "Secret's keys should not be merged": &testCase{ mainClientObjects: []runtime.Object{ &core_v1.Secret{ @@ -896,6 +1040,7 @@ func TestController(t *testing.T) { }, }, } + for name, tc := range testcases { tc := tc t.Run(name, func(t *testing.T) { diff --git a/pkg/controller/resource_sync_task.go b/pkg/controller/resource_sync_task.go index fe6e021..04f3861 100644 --- a/pkg/controller/resource_sync_task.go +++ b/pkg/controller/resource_sync_task.go @@ -50,6 +50,9 @@ type resourceStatusError struct { type resourceInfo struct { actual *unstructured.Unstructured status resourceStatus + + // if actual is a ServiceBinding, we resolve the secret once it's been processed. + serviceBindingSecret *core_v1.Secret } func (ri *resourceInfo) isReady() bool { @@ -155,8 +158,7 @@ func (st *resourceSyncTask) processResource(res *smith_v1.Resource) resourceInfo } // Check if resource is ready - ready, retriable, err := st.rc.IsReady(resUpdated) - if err != nil { + if ready, retriable, err := st.rc.IsReady(resUpdated); err != nil { return resourceInfo{ actual: resUpdated, status: resourceStatusError{ @@ -164,18 +166,48 @@ func (st *resourceSyncTask) processResource(res *smith_v1.Resource) resourceInfo isRetriableError: retriable, }, } - } - if ready { + } else if !ready { return resourceInfo{ actual: resUpdated, - status: resourceStatusReady{}, + status: resourceStatusInProgress{}, } - } else { + } + + // Augment with binding output (used for references) + bindingSecret, err := st.maybeExtractBindingSecret(resUpdated) + if err != nil { return resourceInfo{ actual: resUpdated, - status: resourceStatusInProgress{}, + status: resourceStatusError{ + err: err, + }, } } + + return resourceInfo{ + actual: resUpdated, + status: resourceStatusReady{}, + serviceBindingSecret: bindingSecret, + } +} + +func (st *resourceSyncTask) maybeExtractBindingSecret(obj *unstructured.Unstructured) (*core_v1.Secret, error) { + if obj.GroupVersionKind() != sc_v1b1.SchemeGroupVersion.WithKind("ServiceBinding") { + return nil, nil + } + actual, err := st.scheme.ConvertToVersion(obj, sc_v1b1.SchemeGroupVersion) + if err != nil { + return nil, errors.WithStack(err) + } + serviceBinding := actual.(*sc_v1b1.ServiceBinding) + secret, exists, err := st.store.Get(core_v1.SchemeGroupVersion.WithKind("Secret"), serviceBinding.Namespace, serviceBinding.Spec.SecretName) + if err != nil { + return nil, errors.Wrap(err, "error finding output Secret") + } + if !exists { + return nil, errors.New("cannot find output Secret") + } + return secret.(*core_v1.Secret), nil } func (st *resourceSyncTask) checkAllDependenciesAreReady(res *smith_v1.Resource) resourceStatus { @@ -375,7 +407,7 @@ func (st *resourceSyncTask) prepareDependencies(dependsOn []smith_v1.ResourceNam var err error actual, err = st.scheme.ConvertToVersion(unstructuredActual, gvk.GroupVersion()) if err != nil { - return nil, err + return nil, errors.WithStack(err) } actual.GetObjectKind().SetGroupVersionKind(gvk) } else { diff --git a/pkg/controller/spec_processor.go b/pkg/controller/spec_processor.go index 4fe35ef..7841e8a 100644 --- a/pkg/controller/spec_processor.go +++ b/pkg/controller/spec_processor.go @@ -5,6 +5,7 @@ import ( "reflect" "regexp" "strings" + "unicode/utf8" smith_v1 "github.com/atlassian/smith/pkg/apis/smith/v1" "github.com/atlassian/smith/pkg/resources" @@ -14,6 +15,10 @@ import ( const ( // Separator between reference to a resource by name and JsonPath within a resource ReferenceSeparator = "#" + // Separator between dependency and type of output (i.e. resolve a dependency + // to some produced object) + ResourceOutputSeparator = ":" + ResourceOutputNameBindSecret = "bindsecret" ) var ( @@ -111,7 +116,12 @@ func (sp *SpecProcessor) processMatch(selector string, primitivesOnly bool) (int if len(parts) < 2 { return nil, errors.Errorf("cannot include whole object: %s", selector) } - objName := smith_v1.ResourceName(parts[0]) + objNameParts := strings.SplitN(parts[0], ResourceOutputSeparator, 2) + objName := smith_v1.ResourceName(objNameParts[0]) + resourceOutputName := "" + if len(objNameParts) == 2 { + resourceOutputName = objNameParts[1] + } if objName == sp.selfName { return nil, errors.Errorf("self references are not allowed: %s", selector) } @@ -122,18 +132,40 @@ func (sp *SpecProcessor) processMatch(selector string, primitivesOnly bool) (int if _, allowed := sp.allowedResources[objName]; !allowed { return nil, errors.Errorf("references can only point at direct dependencies: %s", selector) } + + var objToTraverse interface{} + switch resourceOutputName { + case "": + objToTraverse = resInfo.actual.Object + case ResourceOutputNameBindSecret: + if resInfo.serviceBindingSecret == nil { + return nil, errors.Errorf("%q requested, but %q is not a ServiceBinding", resourceOutputName, objName) + } + objToTraverse = resInfo.serviceBindingSecret + default: + return nil, errors.Errorf("resource output name %q not understood for %q", resourceOutputName, objName) + } + // To avoid overcomplicated format of reference like this: {{{res1#{$.a.string}}}} // And have something like this instead: {{{res1#a.string}}} jsonPath := fmt.Sprintf("{$.%s}", parts[1]) - fieldValue, err := resources.GetJsonPathValue(resInfo.actual.Object, jsonPath, false) + fieldValue, err := resources.GetJsonPathValue(objToTraverse, jsonPath, false) if err != nil { return nil, errors.Wrapf(err, "failed to process JsonPath reference %s", selector) } if fieldValue == nil { return nil, errors.Errorf("field not found: %s", selector) } + if primitivesOnly { - switch fieldValue.(type) { + switch typedFieldValue := fieldValue.(type) { + case []byte: + // Secrets are in bytes. We wildly cast them to a string and hope for the best + // so we can put them in the JSON in a 'nice' way. + if !utf8.Valid(typedFieldValue) { + return nil, errors.Errorf("cannot expand non-UTF8 byte array field %q", selector) + } + fieldValue = string(typedFieldValue) case int, uint: case string, bool: case float32, float64: diff --git a/pkg/controller/spec_processor_test.go b/pkg/controller/spec_processor_test.go index 0162426..f4817a9 100644 --- a/pkg/controller/spec_processor_test.go +++ b/pkg/controller/spec_processor_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + core_v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -116,6 +117,33 @@ func TestSpecProcessor(t *testing.T) { assert.Equal(t, expected, obj) } +func TestSpecProcessorBindSecret(t *testing.T) { + t.Parallel() + sp := SpecProcessor{ + selfName: "abc", + resources: processedResources(), + allowedResources: map[smith_v1.ResourceName]struct{}{ + "res1": {}, + "resbinding": {}, + }, + } + obj := map[string]interface{}{ + "ref": map[string]interface{}{ + "int": "{{{res1#a.int}}}", + "secret": "{{resbinding:bindsecret#Data.password}}", + }, + } + expected := map[string]interface{}{ + "ref": map[string]interface{}{ + "int": 42, + "secret": "secret", + }, + } + + require.NoError(t, sp.ProcessObject(obj)) + assert.Equal(t, expected, obj) +} + func TestSpecProcessorErrors(t *testing.T) { t.Parallel() inputs := []struct { @@ -194,6 +222,24 @@ func TestSpecProcessorErrors(t *testing.T) { }, err: `invalid reference at "invalid#[0]": references can only point at direct dependencies: resX#a.string`, }, + { + obj: map[string]interface{}{ + "invalid": "{{res1:bindsecret#Data.password}}", + }, + err: `invalid reference at "invalid": "bindsecret" requested, but "res1" is not a ServiceBinding`, + }, + { + obj: map[string]interface{}{ + "invalid": "{{resbinding:someotherthing#notthere}}", + }, + err: `invalid reference at "invalid": resource output name "someotherthing" not understood for "resbinding"`, + }, + { + obj: map[string]interface{}{ + "invalid": "{{resbinding:bindsecret#Data.nonutf8}}", + }, + err: `invalid reference at "invalid": cannot expand non-UTF8 byte array field "resbinding:bindsecret#Data.nonutf8"`, + }, } for i, input := range inputs { input := input @@ -203,7 +249,8 @@ func TestSpecProcessorErrors(t *testing.T) { selfName: "self1", resources: processedResources(), allowedResources: map[smith_v1.ResourceName]struct{}{ - "res1": {}, + "res1": {}, + "resbinding": {}, }, } assert.EqualError(t, sp.ProcessObject(input.obj), input.err) @@ -240,6 +287,16 @@ func processedResources() map[smith_v1.ResourceName]*resourceInfo { }, status: resourceStatusReady{}, }, + "resbinding": { + actual: &unstructured.Unstructured{}, + status: resourceStatusReady{}, + serviceBindingSecret: &core_v1.Secret{ + Data: map[string][]byte{ + "password": []byte("secret"), + "nonutf8": {255, 254, 255}, + }, + }, + }, "resX": { actual: &unstructured.Unstructured{ Object: map[string]interface{}{