orange-cloudfoundry / terraform-provider-cloudfoundry

A terraform provider to manage a Cloud Foundry instance.
Apache License 2.0
31 stars 8 forks source link

Support for managing feature flags #16

Closed dcarley closed 7 years ago

dcarley commented 7 years ago

I'd like to add support for managing Cloud Foundry feature flags.

They're a bit odd in that, unlike most other Terraform resources, there's nothing to create/destroy because they always exist in some state. It would also be possible for two resources to fight over the same flag if you told Terraform to do so.

I'm interested in some opinions about the implementation. There seems to be three possible options for the schema and user interface. I think I prefer the first or the last:

  1. Single resource, multiple flags, using a map. This requires the least repetition for the user. However it would require dropping support for Terraform 0.7 because non-string map items wasn't added until Terraform 0.8

    resource "cloudfoundry_feature_flags" "feature_flags" { flags { user_org_creation = false diego_docker = true } }

  2. Single resource, multiple flags, using a set. This requires a bit more typing and makes the diff harder to read because the hash of each item changes when the value does.

    resource "cloudfoundry_feature_flags" "feature_flags" { flag { name = "user_org_creation" value = false } flag { name = "diego_docker" value = true } }

  3. Separate resources, single flag each. This is the simplest to implement but requires more repetition and results in more "read" calls against the Cloud Controller API.

    resource "cloudfoundry_feature_flag" "user_org_creation" { name = "user_org_creation" value = false } resource "cloudfoundry_feature_flag" "diego_docker" { name = "diego_docker" value = true }

What do you think?

ArthurHlt commented 7 years ago

Nice feature :)

About the odd you found, you can do a simple thing. Just don't do any action on destroy and when create is called simply call back the update function.

You have actually a fourth solution, this one: Use normal schema, I know that it will be necessary to add manually in schema each new features parameter which will come in Cloud Foundry but I feel more comfortable about have tight control.

This will help the user to know what he can do to set feature flags.

resource "cloudfoundry_feature_flags" "feature_flags" {
   user_org_creation = false
   diego_docker = true
}

Another solution if we want the user to have the ability to set a feature flags which is not yet in the provider is to add a custom_flag (it will pretty look like what you suggest as a first solution) sets, this will look like that:

resource "cloudfoundry_feature_flags" "feature_flags" {
   user_org_creation = false
   diego_docker = true
   custom_flags {
       my_custom_flag = true
   }
}

About support terraform 0.7 (which actually another discussion), you have to know that this provider was created when terraform was in alpha for 0.8 and I saw that the rpc implementation changed. That's why at this time, I support those 2 versions, now it's unnecessary as tf 0.8 is here from few months.

What do you think ? I know that you came with 3 solutions and I've added 2 more and take no decision. For me the last one with custom_flags could be the best one

dcarley commented 7 years ago

I'm unsure about either of those solutions. Hardcoding known feature flags seems like a lot of work in terms of:

The addition of custom_flags would help with ongoing maintenance but it increases the complexity to the equivalent of 1 and 4 combined.

I feel more comfortable about have tight control.

Could you elaborate why?

About the odd you found, you can do a simple thing. Just don't do any action on destroy and when create is called simply call back the update function.

It occurred to me that we could doing something clever by storing the initial unmanaged state on create and restoring it on delete. But I'm really not sure it's necessary.

About support terraform 0.7 (which actually another discussion), you have to know that this provider was created when terraform was in alpha for 0.8 and I saw that the rpc implementation changed. That's why at this time, I support those 2 versions, now it's unnecessary as tf 0.8 is here from few months.

Good to know 👍

ArthurHlt commented 7 years ago

We are actually talking about user experience. If we do like you want:

When user do a typo in the key name with one of your methods

  1. Terraform will not see any errors when verify
  2. It will run the associated api call on cloud foundry
  3. The api call will failed
  4. The state file will be break
  5. All other new or update resources which will need to be performed will not be applied
  6. User have a bad experience :)

Did you also tried intellij on terraform config files ? You have autocompletion based on keys accessible in providers with your idea this can't be used. This last case we can just don't take care but the first case i gave you is a real problem.

And about your idea, yep that's clever, I like it but I also don't know if store that as a file is a good thing

ArthurHlt commented 7 years ago

Implemented in this way in commit https://github.com/orange-cloudfoundry/terraform-provider-cloudfoundry/commit/940eca569ced964aca61ce544cd6a8086d2b7da8 :

resource "cloudfoundry_feature_flags" "feature_flags" {
  diego_docker = true
  // other default flag, look at https://github.com/orange-cloudfoundry/terraform-provider-cloudfoundry/blob/master/resources/feature_flags.go#L15-L29
  custom_flag {
    name = "task_creation"
    enabled = false
  }
}