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

[FEATURE] Re-add Resource databricks_instance_profile skip_validation option #762

Closed
tylangesmith opened this issue Aug 5, 2021 · 12 comments · Fixed by #857
Closed

[FEATURE] Re-add Resource databricks_instance_profile skip_validation option #762

tylangesmith opened this issue Aug 5, 2021 · 12 comments · Fixed by #857
Labels
aws Occurring on AWS cloud wontfix This will not be worked on

Comments

@tylangesmith
Copy link
Contributor

Hey all,

As an enterprise customer we've recently started configuring all of our Databricks configuration using infrastructure as code principles. This has lead us to using this terraform databricks provider.

One of our current frustrations comes from the databricks_instance_profile resource. The 2.0 api specifies that the instance profile is validated using the aws ec2 run-instance --dry-run command.

In practice this sounds like a good idea. However for enterprise customers who have a mature cloud environment there are security postures that are enforced e.g. AWS SCPs that enforce a set of tags to be applied to EC2 instances.

By using the aws ec2 run-instance --dry-run command to validate without the ability to specific additional run options this gives us a false positive validation check. To get around this we can use the skip_validation option provided by the 2.0 api.

As of this provider's 0.3.0 release this skip_validation option was removed from the databricks_instance_profile resource.

Is the removal of this option able to be reconsidered? As it has a very legitimate use-case for us enterprise customers.

@tylangesmith tylangesmith changed the title [FEATURE REQUEST] Re-add Resource databricks_instance_profile skip_validation option [FEATURE] Re-add Resource databricks_instance_profile skip_validation option Aug 5, 2021
@nfx
Copy link
Contributor

nfx commented Aug 5, 2021

How to guarantee that instance profile added would work, if we skip validation? Terraform's goal is to create a resource in a valid runnable state or fail with error and remove that underlying invalid resource.

Can you add exceptions for dry runs?

There were multiple complaints that clusters with attached profiles didn't work.

@tylangesmith
Copy link
Contributor Author

How to guarantee that instance profile added would work, if we skip validation? There were multiple complaints that clusters with attached profiles didn't work.

Can you add exceptions for dry runs?

That's the risk you run when using the skip_validation option which is probably a fairly obvious side effect.

To avoid this you would have the skip_validation flag as optional and set the default to false.

This would not impact current consumers of the resource while also giving the option for users who understand the potential impacts. This also brings the resource more inline with the 2.0 api functionality.

@nfx nfx added the wontfix This will not be worked on label Aug 5, 2021
@nfx
Copy link
Contributor

nfx commented Aug 5, 2021

@tylangesmith provider core team doesn't currently have cycles for this feature request. If you still would like to have this functionality, please send a pull request with the passing output of make test-awsmt and we'll merge that in.

@adionisioFD
Copy link

Hey, came across this issue recently too. We were trying to enforce tags on cluster creation to follow our organization standards using this documentation as reference: https://docs.databricks.com/clusters/configure.html#enforce-mandatory-tags

Having the mandatory tag policy on the cross account role makes validation fail when a new instance profile is added. Seconding the request to skip validation if there's no other way to workaround this while having a tagging policy.

@davehowell
Copy link

@tylangesmith 👋

The aws ec2 run-instances command also has a --tag-specifications option that can pass a list tags to apply to the ec2 or EBS volume(s). For completeness it would probably be better if it did the dry-run validation and actually passed through those tags, considering that you have to specify them in the instance profile to comply with your SCP requirements anyway.

@tylangesmith
Copy link
Contributor Author

Hey @davehowell, small world 😆

Yeah I had this discussion with our Databricks SA, highlighting these exact issues i.e. the current validation implementation will not work for our AWS environment and likely more customers as they mature their cloud environments. Hence why we want to use the skip_validation functionality.

This sort of simple validation is something I've seen as a common theme across a few of the Databricks APIs. Hopefully with their latest round of funding they'll be able to further the maturity and address these types of issues.

Unfortunately for this issue we're 1 abstraction layer higher than the API implementation so we won't be able to change the underlying implementation and how it actually performs the underlying validation logic.

@davehowell
Copy link

@tylangesmith I see, makes sense then. It should be consistent with the other Databricks APIs. If that feature was previously removed it might be easy to cherry pick it out of the history.

@nfx
Copy link
Contributor

nfx commented Aug 25, 2021

What a chat here 😆

@nfx nfx added the aws Occurring on AWS cloud label Aug 25, 2021
@BenMcClainCarpe
Copy link

This is also something that would be great for us. Right now we have to add the instance profile manually to each workspace.

@ricardogaspar2
Copy link

It would be very valuable for us to have this. Otherwise, we have to do it manually via the UI.

@fabiopaiva
Copy link
Contributor

fabiopaiva commented Oct 14, 2021

In the project I'm working on, was introduced a rule to not allow creating an EC2 instance without disk encryption and therefore the instance validation is failing. As we know the instance works for other projects using the same configuration, we consider it's safe to skip the instance validation. As this option is part of Databricks API, I think this should be available in Terraform as well. Sad to hear it got removed, we are adding instance profile manually and importing it to TF state.

@fabiopaiva
Copy link
Contributor

I believe this is what needs to be reverted:

cdef6f9#diff-142ef36d5f14d2cdcd77cb2c75ba58375dcfc4209bc51065ab45835994c14c8fL44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws Occurring on AWS cloud wontfix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants