Add support for HDInsight Cluster with Enterprise Security Package #12866

neil-yechenwei · 2021-08-06T00:58:44Z

Currently, TF HDInsight Cluster doesn't support to integrate with Enterprise Security Package (ESP). So I submitted this PR to implement it.

--- PASS: TestAccHDInsightHadoopCluster_securityProfile (1995.55s)
--- PASS: TestAccHDInsightSparkCluster_securityProfile (2476.50s)
--- PASS: TestAccHDInsightKafkaCluster_securityProfile (2082.18s)
--- PASS: TestAccHDInsightInteractiveQueryCluster_securityProfile (1806.02s)
--- PASS: TestAccHDInsightHBaseCluster_securityProfile (3006.68s)

API Reference:
https://github.com/Azure/azure-rest-api-specs/blob/a331a455fd31e022f8df07105058ae586a02ab03/specification/hdinsight/resource-manager/Microsoft.HDInsight/stable/2018-06-01-preview/cluster.json#L801

Overview:
https://docs.microsoft.com/en-us/azure/hdinsight/domain-joined/apache-domain-joined-configure-using-azure-adds

grayzu · 2021-08-06T21:21:48Z

Thanks for this PR @neil-yechenwei!

Since this is a pretty complicated scenario, can you add your tests to the examples folder so that customers can have a starting point for how to be successfully with these new properties?

neil-yechenwei · 2021-08-09T06:00:54Z

@grayzu , thanks for your comment. The example has been added. Thanks.

manicminer

Hi @neil-yechenwei, thanks for this PR. This is mostly looking good. I've made some comments inline - there's also another PR #13011 which (if merged) will make for simpler configurations.

If you can look at the below, and the tests pass, this should be good to merge. Thanks!

internal/services/hdinsight/hdinsight_hadoop_cluster_resource.go

internal/services/hdinsight/hdinsight_hbase_cluster_resource.go

internal/services/hdinsight/hdinsight_interactive_query_cluster_resource.go

internal/services/hdinsight/hdinsight_kafka_cluster_resource.go

internal/services/hdinsight/hdinsight_spark_cluster_resource.go

internal/services/hdinsight/hdinsight_hadoop_cluster_resource_test.go

internal/services/hdinsight/hdinsight_hbase_cluster_resource_test.go

internal/services/hdinsight/hdinsight_interactive_query_cluster_resource_test.go

internal/services/hdinsight/hdinsight_kafka_cluster_resource_test.go

internal/services/hdinsight/hdinsight_spark_cluster_resource_test.go

manicminer

@neil-yechenwei Thanks for the changes. There are still some crash opportunities, noted below, but the main thing on further thought is that azurerm_active_directory_domain_service has a quota of 1 per tenant or per subscription, so we'll need a way of handling that when parallelizing tests, else it'll conflict with the test in the domainservices package.

internal/services/hdinsight/hdinsight_hadoop_cluster_resource_test.go

manicminer · 2021-08-17T22:04:03Z

internal/services/hdinsight/hdinsight_hadoop_cluster_resource_test.go

+  principal_id         = azurerm_user_assigned_identity.test.principal_id
+}
+
+resource "azurerm_active_directory_domain_service" "test" {


Whilst it should be fine to include an azurerm_active_directory_domain_service in this test, we'll probably have to think about scheduling and parallelization between this test and the test in the domainservices package - it's only possible to have a single domain service at any one time and they take quite a while to stand up and tear down.

If it's possible, could you provide some suggestion on it? Thanks in advance.

I've updated code for your concern. Hopes it's what you expect.

Ah i see. Unfortunately I don't think we can consolidate the tests into one package - the tests really need to live with their respective resources.

I'm looking at the logistics of having a long-running domain service to try and compensate for the concurrency issue, but still working out how to make that work nicely with the domainservices tests themselves.

OK. I separated them into their respective resources and updated TCs related with HDInsight Cluster ESP as parallel TCs. Maybe you have to update your test script in Teamcity to run DomainService's TCs and HDInsight Cluster ESP's TCs in parallel.

examples/hdinsight/enterprise-security-package/main.tf

internal/services/hdinsight/hdinsight_hadoop_cluster_resource.go

internal/services/hdinsight/hdinsight_hbase_cluster_resource.go

internal/services/hdinsight/hdinsight_interactive_query_cluster_resource.go

internal/services/hdinsight/hdinsight_kafka_cluster_resource.go

internal/services/hdinsight/hdinsight_spark_cluster_resource.go

internal/services/hdinsight/validate/hdinsight_cluster_ldaps_urls.go

neil-yechenwei · 2021-08-20T01:47:15Z

@manicminer , kindly ping. May I ask is there any update for above issue?

neil-yechenwei · 2021-08-23T01:29:05Z

@manicminer, For this test issue, is it possible that we can skip these TCs and merge this PR first? Once you figure out this kind of test issue, then enable and update the TCs.

manicminer · 2021-08-23T04:24:05Z

@neil-yechenwei This is challenging because having a persistent AADDS resource will break the tests for that resource. We'll leave this as-is for now, and we'll resolve separately when it becomes a problem - but can we change the AADDS SKU in this tests to Standard?

neil-yechenwei · 2021-08-23T08:46:33Z

@manicminer , Sure. I've changed the AADDS SKU in TCs to Standard. Please have an another look.

internal/services/hdinsight/hdinsight_cluster_resource_test.go

manicminer · 2021-08-25T23:14:11Z

@neil-yechenwei I've refactored the tests a little with a template for the common AADDS configuration. Running the tests again now (expecting them to take around 5-6 hrs), if all goes well this should be good to merge 👍

manicminer · 2021-08-27T09:35:58Z

This is a challenging one to test - a single AADDS test is flaky but 5 successive ones are very prone to failure.

neil-yechenwei · 2021-08-27T12:01:26Z

@manicminer , after checked test log you triggered, seems only one tc "testAccHDInsightInteractiveQueryCluster_securityProfile" is failed in teamcity.
Test log in teamcity:

Per my understanding, I think it's not TF code issue. Hence I just now reran this test case and it can pass on my local. Below is the test result.

Test Result:

$ make testacc TEST=./internal/services/hdinsight/ TESTARGS="-parallel 11 -test.run=TestAccHDInsightCluster_securityProfileSequential" TESTTIMEOUT='1440m'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test ./internal/services/hdinsight/ -v -parallel 11 -test.run=TestAccHDInsightCluster_securityProfileSequential -timeout 1440m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccHDInsightCluster_securityProfileSequential
=== RUN   TestAccHDInsightCluster_securityProfileSequential/interactiveQuery
=== RUN   TestAccHDInsightCluster_securityProfileSequential/interactiveQuery/securityProfile
--- PASS: TestAccHDInsightCluster_securityProfileSequential (8228.09s)
    --- PASS: TestAccHDInsightCluster_securityProfileSequential/interactiveQuery (8228.09s)
        --- PASS: TestAccHDInsightCluster_securityProfileSequential/interactiveQuery/securityProfile (8228.09s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/hdinsight     8234.541s

neil-yechenwei · 2021-08-28T22:55:33Z

@manicminer , after rerun the TCs in TeamCity, the failed test case "testAccHDInsightInteractiveQueryCluster_securityProfile" you encountered can pass now in TeamCity. So I think it's not TF code problem.

neil-yechenwei · 2021-08-30T11:58:59Z

@manicminer , all TCs passed now in TeamCity after rerun.

According to my observation, seems the test script in TeamCity would clean up the existing resources at some time every day. And as you can see, these test cases keep running around 11 hours. The execution duration is long. Probably you encountered the situation that the test script is deleting the existing resources regularly when one of these test cases is running. So one of them fail. Hence these test cases can pass after adjusting the execution time to avoid that time period of data cleaning.

manicminer

Hi @neil-yechenwei, thanks for re-running in TC. I agree this is likely to clash with the cleanup jobs due to the extended test duration. We'll likely have to revisit this to improve the stability whilst complying with the singular quota for AADDS, but in the meantime this should be good to merge 👍

github-actions · 2021-09-02T23:07:20Z

This functionality has been released in v2.75.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2021-10-03T02:07:41Z

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.

neil-yechenwei added 13 commits July 29, 2021 15:49

Add support for HDInsight with AAD Domain Service

190f477

update code

3b853ad

update code

204e7b2

update code

d62dbae

update code

05d049f

update code

7b65c44

update code

489f201

update code

50a68c9

update code

2687525

update code

b437f2c

update code

34f8f5a

update code

6b7b59e

update code

f9acd73

github-actions bot added documentation size/XL labels Aug 6, 2021

neil-yechenwei changed the title ~~Add support for HDInsight Cluster with AAD Domain Service~~ Add support for HDInsight Cluster with Enterprise Security Package Aug 6, 2021

neil-yechenwei added 4 commits August 9, 2021 11:50

Merge remote-tracking branch 'upstream/master' into hdinsightwithaad

89f9da6

update code

1f603f9

update code

03c7274

update code

77f6715

manicminer suggested changes Aug 16, 2021

View reviewed changes

neil-yechenwei added 3 commits August 17, 2021 11:01

update code

2edfeda

update code

667ea5b

update code

5727d19

manicminer suggested changes Aug 17, 2021

View reviewed changes

neil-yechenwei added 2 commits August 18, 2021 13:30

Merge remote-tracking branch 'upstream/main' into hdinsightwithaad

e8f097b

update code

2993ec8

neil-yechenwei force-pushed the hdinsightwithaad branch from ec8f8ef to 7ea70e2 Compare August 18, 2021 13:30

neil-yechenwei requested a review from manicminer August 20, 2021 01:47

manicminer added the waiting-response label Aug 23, 2021

update code

68d9f67

github-actions bot removed the waiting-response label Aug 23, 2021

manicminer reviewed Aug 24, 2021

View reviewed changes

internal/services/hdinsight/hdinsight_cluster_resource_test.go Outdated Show resolved Hide resolved

neil-yechenwei requested a review from manicminer August 24, 2021 13:15

hdinsight test refactoring for security_profile tests

1ba88cf

neil-yechenwei force-pushed the hdinsightwithaad branch 2 times, most recently from 866c877 to 1ba88cf Compare August 28, 2021 00:24

update tc

c747523

update code

c7a2707

manicminer approved these changes Aug 31, 2021

View reviewed changes

manicminer added enhancement service/domain-services service/hdinsight labels Aug 31, 2021

manicminer added this to the v2.75.0 milestone Aug 31, 2021

manicminer merged commit f2f0919 into hashicorp:main Aug 31, 2021

manicminer added a commit that referenced this pull request Aug 31, 2021

Changelog for #12866

7fe2cf3

github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for HDInsight Cluster with Enterprise Security Package #12866

Add support for HDInsight Cluster with Enterprise Security Package #12866

neil-yechenwei commented Aug 6, 2021 •

edited

Loading

grayzu commented Aug 6, 2021

neil-yechenwei commented Aug 9, 2021

manicminer left a comment

manicminer left a comment

manicminer Aug 17, 2021

neil-yechenwei Aug 18, 2021 •

edited

Loading

neil-yechenwei Aug 18, 2021 •

edited

Loading

manicminer Aug 18, 2021

neil-yechenwei Aug 18, 2021 •

edited

Loading

neil-yechenwei commented Aug 20, 2021

neil-yechenwei commented Aug 23, 2021 •

edited

Loading

manicminer commented Aug 23, 2021

neil-yechenwei commented Aug 23, 2021 •

edited

Loading

manicminer commented Aug 25, 2021 •

edited

Loading

manicminer commented Aug 27, 2021

neil-yechenwei commented Aug 27, 2021 •

edited

Loading

neil-yechenwei commented Aug 28, 2021 •

edited

Loading

neil-yechenwei commented Aug 30, 2021 •

edited

Loading

manicminer left a comment

github-actions bot commented Sep 2, 2021

github-actions bot commented Oct 3, 2021

Add support for HDInsight Cluster with Enterprise Security Package #12866

Add support for HDInsight Cluster with Enterprise Security Package #12866

Conversation

neil-yechenwei commented Aug 6, 2021 • edited Loading

grayzu commented Aug 6, 2021

neil-yechenwei commented Aug 9, 2021

manicminer left a comment

Choose a reason for hiding this comment

manicminer left a comment

Choose a reason for hiding this comment

manicminer Aug 17, 2021

Choose a reason for hiding this comment

neil-yechenwei Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

neil-yechenwei Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

manicminer Aug 18, 2021

Choose a reason for hiding this comment

neil-yechenwei Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

neil-yechenwei commented Aug 20, 2021

neil-yechenwei commented Aug 23, 2021 • edited Loading

manicminer commented Aug 23, 2021

neil-yechenwei commented Aug 23, 2021 • edited Loading

manicminer commented Aug 25, 2021 • edited Loading

manicminer commented Aug 27, 2021

neil-yechenwei commented Aug 27, 2021 • edited Loading

neil-yechenwei commented Aug 28, 2021 • edited Loading

neil-yechenwei commented Aug 30, 2021 • edited Loading

manicminer left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 2, 2021

github-actions bot commented Oct 3, 2021

neil-yechenwei commented Aug 6, 2021 •

edited

Loading

neil-yechenwei Aug 18, 2021 •

edited

Loading

neil-yechenwei Aug 18, 2021 •

edited

Loading

neil-yechenwei Aug 18, 2021 •

edited

Loading

neil-yechenwei commented Aug 23, 2021 •

edited

Loading

neil-yechenwei commented Aug 23, 2021 •

edited

Loading

manicminer commented Aug 25, 2021 •

edited

Loading

neil-yechenwei commented Aug 27, 2021 •

edited

Loading

neil-yechenwei commented Aug 28, 2021 •

edited

Loading

neil-yechenwei commented Aug 30, 2021 •

edited

Loading