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

New resource: azurerm_databricks_access_connector #18709

Merged
merged 10 commits into from
Oct 14, 2022

Conversation

favoretti
Copy link
Collaborator

@favoretti favoretti commented Oct 12, 2022

Also upgrade Databricks SDK to 2022-04-01-preview

Fixes #18518

@nfx
Copy link

nfx commented Oct 12, 2022

Thank you so much, @favoretti !

@favoretti
Copy link
Collaborator Author

@tombuildsstuff One catch about this resource. Apparently SPN auth doesn't work with this API, only user-based auth. Wonder what to do about it..

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @favoretti! Since this is still in private preview I expected the tests to fail when trying to create the connector. Unfortunately it's created the connector in a failed state and currently can't be deleted. I'm going to mark this PR as blocked for the time being while we investigate this further.

Also just as an aside all resources within the provider will eventually need to be converted to typed resources for the gradual shift to using Pandora generated ones. Our contributor docs have a section on how to add new typed resources which will hopefully be helpful for future contributions 🙂.

@favoretti
Copy link
Collaborator Author

Thanks @stephybun. I completely blacked out on typed resources, I'll rewrite tomorrow, sorry about that.

That failed connector will eventually be deletable. But the real reason it fails is that API doesn't work with SPN auth. I'm working with @nfx and team to see what the ETA is on supporting that.

If persona authentication is used - resource works.

@nfx
Copy link

nfx commented Oct 13, 2022

@favoretti You say it's the same style of issue as creating the AKV scope for Databricks? 🫣

Also upgrade Databricks SDK to `2022-04-01-preview`
I added acceptance tests, but feature is in private preview.

Fixes hashicorp#18518
@favoretti favoretti force-pushed the favoretti/dbrk_access_connector branch from d393af9 to d9963f1 Compare October 13, 2022 08:48
@favoretti
Copy link
Collaborator Author

@stephybun rewrote this, SPN thing is still under investigation, but can you please have another look code-wise? Thanks!

@favoretti favoretti requested a review from stephybun October 13, 2022 09:25
@favoretti
Copy link
Collaborator Author

@favoretti You say it's the same style of issue as creating the AKV scope for Databricks? 🫣

@nfx Don't think so. AKV scope talks to Databricks DataPlane API, i.e. directly to the instance. This call is directed at management.azure.com, i.e. ARM API. Although you folks would know better, but I think this one should be easier to address.

@favoretti
Copy link
Collaborator Author

I hear SPN auth should be fixed. I'll retest today.

@favoretti
Copy link
Collaborator Author

Local acceptance tests pass, running TC now.

$ TF_ACC=1 go test -v ./internal/services/databricks -timeout=1000m -run='TestAccDatabricksAccessConnector'
=== RUN   TestAccDatabricksAccessConnector_basic
=== PAUSE TestAccDatabricksAccessConnector_basic
=== RUN   TestAccDatabricksAccessConnector_requiresImport
=== PAUSE TestAccDatabricksAccessConnector_requiresImport
=== CONT  TestAccDatabricksAccessConnector_basic
=== CONT  TestAccDatabricksAccessConnector_requiresImport
--- PASS: TestAccDatabricksAccessConnector_basic (164.31s)
--- PASS: TestAccDatabricksAccessConnector_requiresImport (184.18s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/databricks	185.333s

@favoretti
Copy link
Collaborator Author

Failed test isn't related to either upgrade or the changes and seems to be transient. All the rest passes.
image

@favoretti
Copy link
Collaborator Author

@stephybun should be good to go for another review and potential merge. Would you take a look? Thanks a lot!

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor comments on the error messages, other this is good to go!

@favoretti
Copy link
Collaborator Author

Thanks for the suggestions, @stephybun, committed, once unit tests complete, we should be gtg.

@favoretti favoretti requested a review from stephybun October 14, 2022 10:57
Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @favoretti LGTM 🧋

@stephybun stephybun merged commit 54f23b8 into hashicorp:main Oct 14, 2022
stephybun added a commit that referenced this pull request Oct 14, 2022
@nfx
Copy link

nfx commented Oct 14, 2022

🥳

@mvvsubbu
Copy link

mvvsubbu commented Nov 10, 2022

Thanks for this @favoretti! Since this is still in private preview I expected the tests to fail when trying to create the connector. Unfortunately it's created the connector in a failed state and currently can't be deleted. I'm going to mark this PR as blocked for the time being while we investigate this further.

Also just as an aside all resources within the provider will eventually need to be converted to typed resources for the gradual shift to using Pandora generated ones. Our contributor docs have a section on how to add new typed resources which will hopefully be helpful for future contributions 🙂.

I am from Azure Databricks Product group and I see the conversation about creation failing with SPN and Private Preview of the resource. Let me clarify those two issues here.

  1. SPN support for Access Connector is enabled in mid of October.
  2. Access Connector Resource is in Public Preview (Not Private Preview).

We have received a customer escalation about the resource being documented as Private Preview. Can you help us understand the process to update this to Public Preview.

https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/databricks_access_connector

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2022
@stephybun stephybun modified the milestones: Blocked, v3.28.0 Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Databricks Access Connector
4 participants