Provisioning template with generated parameters failed through template broker

[spadgett]

I'm seeing the following error provisioning the template cakephp-mysql-persistent through the service catalog.

BuildConfig "cakephp-mysql-persistent" is invalid: spec.triggers[2].github.secret: Required value

The GitHub secret comes from a generated parameter. It appears the value is not being generated, however.

Here is the Instance request the UI is making to the service catalog:

{
  "kind": "Instance",
  "apiVersion": "servicecatalog.k8s.io/v1alpha1",
  "metadata": {
    "name": "cakephp-mysql-persistent-c0l24",
    "generateName": "cakephp-mysql-persistent-",
    "namespace": "cake-php",
    "selfLink": "/apis/servicecatalog.k8s.io/v1alpha1/namespaces/cake-php/instances/cakephp-mysql-persistent-c0l24",
    "uid": "05346697-4700-11e7-9c59-0242ac110002",
    "resourceVersion": "41",
    "creationTimestamp": "2017-06-01T19:25:07Z",
    "finalizers": [
      "kubernetes"
    ]
  },
  "spec": {
    "serviceClassName": "cakephp-mysql-persistent",
    "planName": "default",
    "parameters": {
      "APPLICATION_DOMAIN": "",
      "CAKEPHP_SECRET_TOKEN": "",
      "CAKEPHP_SECURITY_CIPHER_SEED": "",
      "CAKEPHP_SECURITY_SALT": "",
      "COMPOSER_MIRROR": "",
      "CONTEXT_DIR": "",
      "DATABASE_ENGINE": "mysql",
      "DATABASE_NAME": "default",
      "DATABASE_PASSWORD": "",
      "DATABASE_SERVICE_NAME": "mysql",
      "DATABASE_USER": "cakephp",
      "GITHUB_WEBHOOK_SECRET": "",
      "MEMORY_LIMIT": "512Mi",
      "MEMORY_MYSQL_LIMIT": "512Mi",
      "NAME": "cakephp-mysql-persistent",
      "NAMESPACE": "openshift",
      "OPCACHE_REVALIDATE_FREQ": "2",
      "SOURCE_REPOSITORY_REF": "",
      "SOURCE_REPOSITORY_URL": "https://github.com/openshift/cakephp-ex.git",
      "VOLUME_CAPACITY": "1Gi",
      "template.openshift.io/namespace": "cake-php",
      "template.openshift.io/requester-username": "sam"
    },
    "externalID": "31704df1-d34d-488c-87ec-005d4ad77884"
  },
  "status": {
    "conditions": [],
    "asyncOpInProgress": false
  }
}

Here is the TemplateInstance that the broker creates:

TemplateInstance YAML

```yaml apiVersion: template.openshift.io/v1 kind: TemplateInstance metadata: name: 31704df1-d34d-488c-87ec-005d4ad77884 namespace: cake-php selfLink: >- /apis/template.openshift.io/v1/namespaces/cake-php/templateinstances/31704df1-d34d-488c-87ec-005d4ad77884 uid: 053e4927-4700-11e7-bccb-3a3fa09a4394 resourceVersion: '16788' creationTimestamp: '2017-06-01T19:25:07Z' spec: template: metadata: name: cakephp-mysql-persistent namespace: openshift selfLink: >- /apis/template.openshift.io/v1/namespaces/openshift/templates/cakephp-mysql-persistent uid: a4255703-46d6-11e7-8a34-3a3fa09a4394 resourceVersion: '895' creationTimestamp: '2017-06-01T14:28:54Z' annotations: description: >- An example CakePHP application with a MySQL database. For more information about using this template, including OpenShift considerations, see https://github.com/openshift/cakephp-ex/blob/master/README.md. iconClass: icon-php openshift.io/display-name: CakePHP + MySQL (Persistent) tags: 'quickstart,php,cakephp' template.openshift.io/documentation-url: 'https://github.com/openshift/cakephp-ex' template.openshift.io/long-description: >- This template defines resources needed to develop a CakePHP application, including a build configuration, application deployment configuration, and database deployment configuration. template.openshift.io/provider-display-name: 'Red Hat, Inc.' template.openshift.io/support-url: 'https://access.redhat.com' message: >- The following service(s) have been created in your project: ${NAME}, ${DATABASE_SERVICE_NAME}. For more information about using this template, including OpenShift considerations, see https://github.com/openshift/cake-ex/blob/master/README.md. objects: - kind: Secret apiVersion: v1 metadata: name: '${NAME}' stringData: database-user: '${DATABASE_USER}' database-password: '${DATABASE_PASSWORD}' cakephp-secret-token: '${CAKEPHP_SECRET_TOKEN}' cakephp-security-salt: '${CAKEPHP_SECURITY_SALT}' cakephp-security-cipher-seed: '${CAKEPHP_SECURITY_CIPHER_SEED}' - kind: Service apiVersion: v1 metadata: name: '${NAME}' annotations: description: Exposes and load balances the application pods service.alpha.openshift.io/dependencies: '[{"name": "${DATABASE_SERVICE_NAME}", "kind": "Service"}]' spec: ports: - name: web port: 8080 targetPort: 8080 selector: name: '${NAME}' - kind: Route apiVersion: v1 metadata: name: '${NAME}' spec: host: '${APPLICATION_DOMAIN}' to: kind: Service name: '${NAME}' - kind: ImageStream apiVersion: v1 metadata: name: '${NAME}' annotations: description: Keeps track of changes in the application image - kind: BuildConfig apiVersion: v1 metadata: name: '${NAME}' annotations: description: Defines how to build the application spec: source: type: Git git: uri: '${SOURCE_REPOSITORY_URL}' ref: '${SOURCE_REPOSITORY_REF}' contextDir: '${CONTEXT_DIR}' strategy: type: Source sourceStrategy: from: kind: ImageStreamTag namespace: '${NAMESPACE}' name: 'php:7.0' env: - name: COMPOSER_MIRROR value: '${COMPOSER_MIRROR}' output: to: kind: ImageStreamTag name: '${NAME}:latest' triggers: - type: ImageChange - type: ConfigChange - type: GitHub github: secret: '${GITHUB_WEBHOOK_SECRET}' postCommit: script: ./lib/Cake/Console/cake test app AllTests - kind: DeploymentConfig apiVersion: v1 metadata: name: '${NAME}' annotations: description: Defines how to deploy the application server spec: strategy: type: Recreate recreateParams: pre: failurePolicy: Retry execNewPod: command: - ./migrate-database.sh containerName: cakephp-mysql-persistent triggers: - type: ImageChange imageChangeParams: automatic: true containerNames: - cakephp-mysql-persistent from: kind: ImageStreamTag name: '${NAME}:latest' - type: ConfigChange replicas: 1 selector: name: '${NAME}' template: metadata: name: '${NAME}' labels: name: '${NAME}' spec: containers: - name: cakephp-mysql-persistent image: ' ' ports: - containerPort: 8080 readinessProbe: timeoutSeconds: 3 initialDelaySeconds: 3 httpGet: path: /health.php port: 8080 livenessProbe: timeoutSeconds: 3 initialDelaySeconds: 30 httpGet: path: / port: 8080 env: - name: DATABASE_SERVICE_NAME value: '${DATABASE_SERVICE_NAME}' - name: DATABASE_ENGINE value: '${DATABASE_ENGINE}' - name: DATABASE_NAME value: '${DATABASE_NAME}' - name: DATABASE_USER valueFrom: secretKeyRef: name: '${NAME}' key: database-user - name: DATABASE_PASSWORD valueFrom: secretKeyRef: name: '${NAME}' key: database-password - name: CAKEPHP_SECRET_TOKEN valueFrom: secretKeyRef: name: '${NAME}' key: cakephp-secret-token - name: CAKEPHP_SECURITY_SALT valueFrom: secretKeyRef: name: '${NAME}' key: cakephp-security-salt - name: CAKEPHP_SECURITY_CIPHER_SEED valueFrom: secretKeyRef: name: '${NAME}' key: cakephp-security-cipher-seed - name: OPCACHE_REVALIDATE_FREQ value: '${OPCACHE_REVALIDATE_FREQ}' resources: limits: memory: '${MEMORY_LIMIT}' - kind: PersistentVolumeClaim apiVersion: v1 metadata: name: '${DATABASE_SERVICE_NAME}' spec: accessModes: - ReadWriteOnce resources: requests: storage: '${VOLUME_CAPACITY}' - kind: Service apiVersion: v1 metadata: name: '${DATABASE_SERVICE_NAME}' annotations: description: Exposes the database server spec: ports: - name: mysql port: 3306 targetPort: 3306 selector: name: '${DATABASE_SERVICE_NAME}' - kind: DeploymentConfig apiVersion: v1 metadata: name: '${DATABASE_SERVICE_NAME}' annotations: description: Defines how to deploy the database spec: strategy: type: Recreate triggers: - type: ImageChange imageChangeParams: automatic: true containerNames: - mysql from: kind: ImageStreamTag namespace: '${NAMESPACE}' name: 'mysql:5.7' - type: ConfigChange replicas: 1 selector: name: '${DATABASE_SERVICE_NAME}' template: metadata: name: '${DATABASE_SERVICE_NAME}' labels: name: '${DATABASE_SERVICE_NAME}' spec: volumes: - name: '${DATABASE_SERVICE_NAME}-data' persistentVolumeClaim: claimName: '${DATABASE_SERVICE_NAME}' containers: - name: mysql image: ' ' ports: - containerPort: 3306 volumeMounts: - name: '${DATABASE_SERVICE_NAME}-data' mountPath: /var/lib/mysql/data readinessProbe: timeoutSeconds: 1 initialDelaySeconds: 5 exec: command: - /bin/sh - '-i' - '-c' - >- MYSQL_PWD='${DATABASE_PASSWORD}' mysql -h 127.0.0.1 -u ${DATABASE_USER} -D ${DATABASE_NAME} -e 'SELECT 1' livenessProbe: timeoutSeconds: 1 initialDelaySeconds: 30 tcpSocket: port: 3306 env: - name: MYSQL_USER valueFrom: secretKeyRef: name: '${NAME}' key: database-user - name: MYSQL_PASSWORD valueFrom: secretKeyRef: name: '${NAME}' key: database-password - name: MYSQL_DATABASE value: '${DATABASE_NAME}' resources: limits: memory: '${MEMORY_MYSQL_LIMIT}' parameters: - name: NAME displayName: Name description: >- The name assigned to all of the frontend objects defined in this template. value: cakephp-mysql-persistent required: true - name: NAMESPACE displayName: Namespace description: The OpenShift Namespace where the ImageStream resides. value: openshift required: true - name: MEMORY_LIMIT displayName: Memory Limit description: Maximum amount of memory the CakePHP container can use. value: 512Mi required: true - name: MEMORY_MYSQL_LIMIT displayName: Memory Limit (MySQL) description: Maximum amount of memory the MySQL container can use. value: 512Mi required: true - name: VOLUME_CAPACITY displayName: Volume Capacity description: 'Volume space available for data, e.g. 512Mi, 2Gi' value: 1Gi required: true - name: SOURCE_REPOSITORY_URL displayName: Git Repository URL description: The URL of the repository with your application source code. value: 'https://github.com/openshift/cakephp-ex.git' required: true - name: SOURCE_REPOSITORY_REF displayName: Git Reference description: >- Set this to a branch name, tag or other ref of your repository if you are not using the default branch. - name: CONTEXT_DIR displayName: Context Directory description: >- Set this to the relative path to your project if it is not in the root of your repository. - name: APPLICATION_DOMAIN displayName: Application Hostname description: >- The exposed hostname that will route to the CakePHP service, if left blank a value will be defaulted. - name: GITHUB_WEBHOOK_SECRET displayName: GitHub Webhook Secret description: A secret string used to configure the GitHub webhook. generate: expression from: '[a-zA-Z0-9]{40}' - name: DATABASE_SERVICE_NAME displayName: Database Service Name value: mysql required: true - name: DATABASE_ENGINE displayName: Database Engine description: 'Database engine: postgresql, mysql or sqlite (default).' value: mysql required: true - name: DATABASE_NAME displayName: Database Name value: default required: true - name: DATABASE_USER displayName: Database User value: cakephp required: true - name: DATABASE_PASSWORD displayName: Database Password generate: expression from: '[a-zA-Z0-9]{16}' - name: CAKEPHP_SECRET_TOKEN displayName: CakePHP secret token description: Set this to a long random string. generate: expression from: '[\w]{50}' - name: CAKEPHP_SECURITY_SALT displayName: CakePHP Security Salt description: Security salt for session hash. generate: expression from: '[a-zA-Z0-9]{40}' - name: CAKEPHP_SECURITY_CIPHER_SEED displayName: CakePHP Security Cipher Seed description: Security cipher seed for session hash. generate: expression from: '[0-9]{30}' - name: OPCACHE_REVALIDATE_FREQ displayName: OPcache Revalidation Frequency description: >- How often to check script timestamps for updates, in seconds. 0 will result in OPcache checking for updates on every request. value: '2' - name: COMPOSER_MIRROR displayName: Custom Composer Mirror URL description: The custom Composer mirror URL labels: template: cakephp-mysql-persistent secret: name: 31704df1-d34d-488c-87ec-005d4ad77884 requester: username: sam status: conditions: - type: InstantiateFailure status: 'True' lastTransitionTime: '2017-06-01T19:25:07Z' reason: Failed message: >- BuildConfig "cakephp-mysql-persistent" is invalid: spec.triggers[2].github.secret: Required value ```

cc @jim-minter @jwforres @bparees

[jim-minter]

Tricky. It looks like the broker received the explicit parameter "GITHUB_WEBHOOK_SECRET": "", so it populated the value accordingly rather than generating it. It's equivalent to oc process cakephp-mysql-persistent -p GITHUB_WEBHOOK_SECRET= being run instead of oc process cakephp-mysql-persistent. I think the current broker behaviour is at least consistent with other parts of the template API, and that it would be best if no parameter were sent by the catalog when autogeneration is required. Is this achievable?

[jim-minter]

I recognise that at the moment the catalog has no way of specifically detecting fields that will be autogenerated, but it could in principle recognise that these are not required values, as at least this is indicated in the schema. If it were not to send empty values for these fields, might that be the best interim solution?

[spadgett]

It looks like JSON schema can contain extra properties not defined in the spec.

A JSON Schema MAY contain properties which are not schema keywords. Unknown keywords SHOULD be ignored.

http://json-schema.org/latest/json-schema-core.html

If you could annotate the parameters that are generated fields in the schema somehow, we can handle them specially in the UI. This would allows us to add the "generated if empty" placeholder that we have in our current template form as well.

[spadgett]

Also from the spec

Implementations MAY define additional keywords to JSON Schema. Save for explicit agreement, schema authors SHALL NOT expect these additional keywords to be supported by peer implementations. Implementations SHOULD ignore keywords they do not support.

[spadgett]

@jim-minter OK I see your point about required. If the schema doesn't say the property is required, it can be missing from the object entirely and still validate. Brokers should handle that.

@jwforres Any objections to leaving out empty, optional values from the instance request?

I think we should still look at adding something to indicate that the value will be generated though so we can tell the user.

[jim-minter]

I agree that indicating that the value can be generated by the back-end is a nice to have, but there's not a super quick way to implement this because the json schema go library we're using doesn't neatly support adding additional keywords. Also, note that if we start using a new keyword, the chances of it being used by other brokers may be pretty limited. So I think that leaving out empty, optional values is the best first step here. At this point, should this issue go to a trello card @bparees?

[bparees]

So I think that leaving out empty, optional values is the best first step here. At this point, should this issue go to a trello card @bparees?

@jim-minter have we already updated the broker to report template parameters that are marked in the template as "generated" and "required" as "optional" in the parameter schema?

[jim-minter]

@bparees good point. https://github.com/openshift/origin/pull/14488. Now, what about the rest @bparees ?

[bparees]

1) we should report 'required but generatable" parameters as optional. @jim-minter's PR will do that. 2) the web console should not send back optional parameters that have an empty string value 3) we should open a trello card to annotate "generatable" parameters so the web console knows they are generatable. Agree that this is something it might be better to have consensus on across brokers, so maybe open an issue against the service broker api spec repo first and let's discuss it there?

[spadgett]

(2) is done by https://github.com/openshift/origin-web-catalog/pull/231

[jwforres]

On Tue, Jun 6, 2017 at 9:48 AM, Ben Parees notifications@github.com wrote:

1. we should report 'required but generatable" parameters as optional. @jim-minter https://github.com/jim-minter's PR will do that.
2. the web console should not send back optional parameters that have an empty string value
3. we should open a trello card to annotate "generatable" parameters so the web console knows they are generatable. Agree that this is something it might be better to have consensus on across brokers, so maybe open an issue against the service broker api spec repo first and let's discuss it there?

Agree we should open the issue to have the discussion, but I suspect the result will be that there is a recommended convention that is not part of the spec. That is the status for everything underneath service and plan metadata and those are arguably more spec worthy than generatable params.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openshift/origin/issues/14445#issuecomment-306491356, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZk7SFlf4zqTL4Vsv3S2R3UnJo9G-D1ks5sBViQgaJpZM4NtcFd .

[bparees]

Agree we should open the issue to have the discussion, but I suspect the result will be that there is a recommended convention that is not part of the spec.

yeah i don't expect this to land in the spec, but if we can get some interested parties to agree on a suitable convention, at least we won't be going it alone.

[bparees]

issue opened here: https://github.com/openservicebrokerapi/servicebroker/issues/219

[bparees]

action items have been resolved or moved to other issues.