Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug where binding data is expected to overwrite the related resource name #515

Merged
merged 12 commits into from
Jul 16, 2020

Conversation

isutton
Copy link
Contributor

@isutton isutton commented Jun 17, 2020

Motivation

A bug has been introduced in #407, changing the existing behavior where the data collected from the resource indicated by the annotation (either a corev1.ConfigMap or corev1.Secret) should also replace the field indicating the resource name with the collected data.

Let's take a look in the following example resource:

metadata:
  annotations:
    servicebindingoperator.redhat.io/status.secretName: binding:env:object:secret
status:
  secretName: the-secret

Before #407, when status.secretName was used in a custom environment variable template, it would contain the whole data section of the related Secret, as the following excerpt from #475:

"status": {
  "secretName": {
    "apikey": "XXXX",
    "iam_apikey_description": "Auto-generated for key 0aef56aa-fbb2-4dd7-8bc7-972302e14573",
    "iam_apikey_name": "thetranslator5",
    "iam_role_crn": "crn:v1:bluemix:public:iam::::serviceRole:Manager",
    "iam_serviceid_crn": "crn:v1:bluemix:public:iam-identity::a/ef6a34810cbcd892507d3ebe01e3d95a::serviceid:ServiceId-71e18dc7-1d02-4704-b167-be29f4172044",
    "url": "https://api.us-south.language-translator.watson.cloud.ibm.com/instances/78abe198-feb8-41c9-b293-38b21beb8973"
  },
}

And after #407, the following results were obtained instead:

"status": {
  "secretName": "thetranslator5",
}

Closes #475

Changes

The contents of a related resource are collected, grouped and made available to the custom environment variable template engine; as an example, if the field path .status.dbCredentials represents a resource name, it is expected the related resource contents (more specifically the .data field) to be available under the same .status.dbCredentials path when composing a custom environment variable template in a Service Binding Request.

Below there is an example of what the input object (the service resource that contains the reference to the related resource) and the output object (the resource that is available to the custom environment template context):

// inputObj is whatever object containing both annotations (either directly in it or through its 
// CRD or CSV.
inputObj := map[string]interface{}{
    "metadata": map[string]interface{}{
        // annotations create the relationship of .status.secretName to a secret
        "annotations": map[string]string{....},
    },
    "status": map[string]interface{}{
        // secretName contains the name of a resource living in the same namespace as the 
        // Service Binding Request.
        "secretName": "thetranslator5",
    },
}

// outputObj is inputPath with '.status.dbCredentials' 
outputObj := map[string]interface{}{
    "status": map[string]interface{}{
        "secretName": map[string]interface{}{
          "apikey": "XXXX",
          "iam_apikey_description": "Auto-generated for key 0aef56aa-fbb2-4dd7-8bc7-972302e14573",
          "iam_apikey_name": "thetranslator5",
          "iam_role_crn": "crn:v1:bluemix:public:iam::::serviceRole:Manager",
          "iam_serviceid_crn": "crn:v1:bluemix:public:iam-identity::a/ef6a34810cbcd892507d3ebe01e3d95a::serviceid:ServiceId-71e18dc7-1d02-4704-b167-be29f4172044",
          "url": "https://api.us-south.language-translator.watson.cloud.ibm.com/instances/78abe198-feb8-41c9-b293-38b21beb8973",
        },
    },
}

For further more details refer the CONTRIBUTING.md

@sbose78
Copy link
Member

sbose78 commented Jun 17, 2020

Could you outline the user-facing scenario with an example please?

@isutton
Copy link
Contributor Author

isutton commented Jun 17, 2020

/retest

@isutton
Copy link
Contributor Author

isutton commented Jun 18, 2020

/retest

@sbose78
Copy link
Member

sbose78 commented Jun 19, 2020

Could you update the description with an example of what's broken, that's being fixed?

@isutton
Copy link
Contributor Author

isutton commented Jun 19, 2020

Could you update the description with an example of what's broken, that's being fixed?

@sbose78 updated.

@isutton
Copy link
Contributor Author

isutton commented Jun 19, 2020

/retest

@sbose78
Copy link
Member

sbose78 commented Jun 20, 2020

@isutton Is this an issue with consumption in custom environment variables?

@qibobo
Copy link
Contributor

qibobo commented Jun 23, 2020

@isutton
There are conflicts and needs rebase.

@pratikjagrut
Copy link
Contributor

/retest

@isutton
Copy link
Contributor Author

isutton commented Jul 13, 2020

/retest

Scenarios where the SBR is created prior the application and/or service
are not well supported yet. Currently annotations added to either
service and application are used to correlate those resources, but those
are added by the SBO when the SBR is reconciled; without this
information currently it is not possible to correlate the resource to a
SBR.
@isutton
Copy link
Contributor Author

isutton commented Jul 13, 2020

/test all

@qibobo
Copy link
Contributor

qibobo commented Jul 16, 2020

@isutton
Will this PR be merged soon? PR521 needs rebase after this PR is merged.

@DhritiShikhar
Copy link
Contributor

/lgtm
/approve

@DhritiShikhar
Copy link
Contributor

/hold

@DhritiShikhar
Copy link
Contributor

/lgtm
/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DhritiShikhar

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:

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

@DhritiShikhar
Copy link
Contributor

/hold cancel

@openshift-merge-robot openshift-merge-robot merged commit f95d490 into redhat-developer:master Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Service binding global and service envVarPrefix doesn't behave as expected
8 participants