[FR] User provided variables do not overwrite plan default variables

[claassen]

Description

When provisioning a service instance any user provided variables are ignored if they are also specified by the plan.

From a comment in the broker code here: https://github.com/cloudfoundry-incubator/cloud-service-broker/blob/aeeda52721f507e846b2730d5e597cf272b2efbd/pkg/broker/service_definition.go#L352

// ProvisionVariables gets the variable resolution context for a provision request. // Variables have a very specific resolution order, and this function populates the context to preserve that. // The variable resolution order is the following: // // 1. Variables defined in your computed_variables JSON list. // 2. Variables defined by the selected service plan in its service_properties map. // 3. Variables overridden in the plan's provision_overrides map. // 4. User defined variables (in update_input_variables) // 5. User defined variables (in provision_input_variables or bind_input_variables) // 6. Operator default variables loaded from the environment. // 7. Global operator default variables loaded from the environment. // 8. Default variables (in provision_input_variables or bind_input_variables).

It would appear the desired behaviour is to have user provided variables (4, 5) take precedence over values specified by the plan (2, 3?), but the code does not seem to exhibit this behaviour.

Expected Behavior

User provided variables should take precedence over plan defaults.

I would have assumed that the precedence order described above means that higher numbers take precedence over lower numbers in order, maybe I am interpreting this wrong?

Actual Behavior

Plan default variables take precedence over user provided variables.

Possible Fix

I believe the area of code that would need to be changed is here: https://github.com/cloudfoundry-incubator/cloud-service-broker/blob/aeeda52721f507e846b2730d5e597cf272b2efbd/pkg/broker/service_definition.go#L372

Steps to Reproduce

1. Define a plan with default parameters configured. E.g. csb-azure-mssql-db:

service:
  csb-azure-mssql-db:
    provision:
      defaults: '{ 
        "skip_provider_registration": true,
        "server": "primary"
      }'
    plans: '[
      {
        "name": "StandardS3",
        "id": "a0257493-95b0-488f-8550-78658d31f9d4",
        "description": "StandardS0 DTU based plan",
        "sku_name": "S3",
        "max_storage_gb": 250
      }
   ]'

2. Create a service instance, specifying a custom value for a parameter:

cf create-service csb-azure-mssql-db StandardS0 my-service -c '{"max_storage_gb": 500 }'

3. Check that storage size for the created database (it will be 250 instead of the expected 500)

Context

Prevents users from increasing the storage size of their csb-azure-mssql-db databases via the 'max_storage_gb' parameter.

Your Environment

• Version used: 0.2.8
• Platform (Azure/AWS/GCP): Azure
• Applicable Services: All

[kelvin-j-li]

Can we also support the override of user provided param in 'cf update-service'?

Thanks!

[FelisiaM]

Thanks for raising this.

It would appear the desired behaviour is to have user-provided variables (4, 5) take precedence over values specified by the plan (2, 3?), but the code does not seem to exhibit this behaviour.

Steps 2 and 3 refer to provision defaults that are set at a broker level for all plans within a service. Those ones are overridden by the user input parameters in steps 4 and/or 5.

However, it looks like you are referring to values that are overridden by step 8 which come from plan_input defined in the brokerpak manifest or plan configuration time. As documented here these can be defined as Plan Configuration Parameters but are not listed as Configuration Parameters Here you can find (what I think is) a simpler explanation of the order the properties are merged.

This is the expected behaviour and the intention is to offer the operator/broker author the ability to create plans with pre-defined properties which the developers cannot change (neither at create nor update time). E.g. a plan medium should be the same size for everyone.

My understanding of your request is: Allow the configuration of plans with predefined properties which can be overridden during service instance creation/update time. A few more questions that would help us prioritise this feature and make it right are:

1. What are the service offerings that are impacted by this? Is it just csb-azure-mssql-db or are you wanting this behaviour for all services?
2. Would you be ok with the user input overriding all the properties defined in the plan or would you like to have the ability to still have properties that cannot be changed for a plan during creation/update? E.g when defining this plan should the users also be able to change the SKU as well? Would that still be considered a StandardS3 plan?

plans: '[
    {
      "name": "StandardS3",
      "id": "a0257493-95b0-488f-8550-78658d31f9d4",
      "description": "StandardS0 DTU based plan",
      "sku_name": "S3",
      "max_storage_gb": 250
    }
 ]'

Note/workaround: If you define a plan without any properties, the default values for each of the properties will be used on the creation and the user can define their own values for the properties E.g the following plan will be created by default with max_storage_gb set to 5 unless the user defines max_storage_gb in -c params in create/update request. Is this a solution you can use instead?

plans: '[
    {
      "name": "StandardS3",
      "id": "a0257493-95b0-488f-8550-78658d31f9d4",
      "description": "StandardS0 DTU based plan",
      "sku_name": "S3",
    }
 ]'

Thank, Felisia

[claassen]

@FelisiaM We are looking primarily just to allow users to specify custom storage size beyond the defaults for Azure SQL, but let me test out omitting the storage size from the plan and see if that will work for us.

[claassen]

Omitting the storage size from the plans works in terms of allowing users to specify it, but introduces a new problem which is that the current default in the brokerpak is 5GB, which means even for very high tier plans like P15 the user would only get 5GB of storage, and I don't think there is any sensible default value that could be specified that would work across all plans.

The ideal solution here would be to allow certain fields to have a default value specified by plans, but to also allow them to be overridden by users when provisioning instances.

[FelisiaM]

Thanks for trying it out and for the feedback @claassen .

I'm going to have a chat with the team and see what is the best way to approach this feature and will let you know about the progress.

In the meantime would you mind also clarifying if you be ok with the user having the ability to override all the properties defined in the plan or would you like to have the ability to still have properties that cannot be changed for a plan during creation/update? E.g when defining this plan should the users also be able to change the SKU as well? Would that still be considered a StandardS3 plan?

Thanks, Felisia

[claassen]

Hi @FelisiaM we would definitely need some properties to still not be able to be overridden by users, for example TLS settings and firewall rules.

[kelvin-j-li]

Hi @FelisiaM, Is there any rough timeline for the fix of this open issue?

Thanks.

Kelvin

[piyalibanerjee]

Hi @claassen and @kelvin-j-li -- thank you for submitting this feature request! Before committing to making a change to the CSB to allow developers to override plan defaults such as storage, there's some validation we need to do with other users to see the impact of giving developers that autonomy. From my understanding, it was an intentional decision to not permit overriding plan defaults with the main workaround being for the operators/service managers to create custom plans that the developers can consume. Have you all been creating custom plans as well and would that work for the time being?

[blgm]

Closing due to lack of activity