chore(azurerm): upgrade to 3.3.0 #157

eyenx · 2022-04-22T11:38:35Z

Fixes #145
Fixes #156

Changes proposed in the pull request:

Upgrade azurerm to 3.3.0 following the upgrade guide https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/guides/3.0-upgrade-guide#resource-azurerm_kubernetes_cluster

Tests succesful:

TestTerraformBasicExample 2022-04-22T11:25:27Z command.go:96: Destroy complete! Resources: 18 destroyed.
--- PASS: TestTerraformBasicExample (1473.05s)
PASS
ok      terraform-azurerm-aks/test      1473.061s

Signed-off-by: Toni Tauro [email protected]

Signed-off-by: Toni Tauro <[email protected]>

ghost · 2022-04-22T11:38:47Z

All CLA requirements met.

eyenx · 2022-05-20T10:25:13Z

Hi Reviewers

It has been a month. Can we get some review on this?

@lonegunmanb

lonegunmanb · 2022-05-30T15:55:28Z

Hello @eyenx , thanks for opening this pr. Upgrading to 3.x may introduce some breaking changes, while we're planning on adding a new CI pipeline to this module so the further pr can be tested in GitHub action. We're working on this so the next release won't change module's code dramatically. I'm sorry to say that we might need more time to merge this pr.

lonegunmanb

When apply test/fixture folder, the following errors occured:

╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 37:
│   37: output "test_client_key" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 41:
│   41: output "test_client_certificate" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 45:
│   45: output "test_cluster_ca_certificate" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 49:
│   49: output "test_host" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 53:
│   53: output "test_username" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵

variables.tf

outputs.tf

lonegunmanb · 2022-06-06T07:09:48Z

main.tf

-        subnet_id    = var.ingress_application_gateway_subnet_id
-      }
+  dynamic "ingress_application_gateway" {
+    for_each = var.enable_ingress_application_gateway == null ? [] : ["ingress_application_gateway"]


What if var.enable_ingress_application_gateway is false?

I did the same as on line 80 with identity. As far as I can see we do not check there either if it's null or false

lonegunmanb · 2022-06-06T07:16:46Z

main.tf

-    oms_agent {
-      enabled                    = var.enable_log_analytics_workspace
+  dynamic "oms_agent" {
+    for_each = var.enable_log_analytics_workspace ? ["oms_agent"] : []


What if var.enable_log_analytics_workspace is null? Should we add nullable = false to the variable?

I did the same as on line 80 with identity. As far as I can see we do not check there either if it's null or false

lonegunmanb · 2022-06-07T03:28:39Z

Hi @eyenx , after discussed with my team, we decided to accept this pr before we introduce pipeline to this module. There're some suggestions remain, would you please take a look? Thanks.

Co-authored-by: lonegunmanb <[email protected]>

eyenx · 2022-06-15T09:12:25Z

Hi @lonegunmanb thanks for the review.

I answered some comments of yours and applied some changes.

lonegunmanb · 2022-06-15T12:18:53Z

@eyenx thanks for updating, after run the test I've got the following error:

╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 37:
│   37: output "test_client_key" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 41:
│   41: output "test_client_certificate" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 45:
│   45: output "test_cluster_ca_certificate" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 49:
│   49: output "test_host" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 53:
│   53: output "test_username" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 57:
│   57: output "test_password" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only internal, Terraform requires that any root module output     
│ containing sensitive data be explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding the following argument:
│     sensitive = true
╵

I think it's ok to leave the variables' validation blank for now since the existing variables are lacking of proper validation, I'll deal with them in the later pr. Would you please update the outputs.tf in the test/fixture folder to make the test green light? Thanks.

eyenx · 2022-06-15T12:57:48Z

Done! @lonegunmanb

lonegunmanb · 2022-06-15T23:38:46Z

@eyenx apology for the confusion, but these errors are complaining that the outputs in test/fixture folder are lacking of sensitive argument. Would you please correct these issues? Thanks.

eyenx · 2022-06-16T06:22:51Z

I see should I revert my changes then?

Signed-off-by: Toni Tauro <[email protected]>

eyenx · 2022-06-16T07:58:32Z

@lonegunmanb can you retry? my tests worked now.

lonegunmanb · 2022-06-16T09:28:09Z

Thanks @eyenx, LGTM!

davidkarlsen · 2022-06-16T12:45:16Z

@lonegunmanb Awesome! Will there be a release soon?

lonegunmanb · 2022-06-16T23:20:00Z

@lonegunmanb Awesome! Will there be a release soon?

Hi @davidkarlsen , since this release will introduce breaking changes, I'd like to submit several more pull requests which contain breaking changes to improve the current code so the users will experience one time breaking change then everything will be stable.

And I still need more time to test the impact caused by this major version upgrade, we may warn our users on what will happen if they choose to upgrade this major version.

hairmare · 2022-07-26T08:47:58Z

Hi @lonegunmanb Thanks for merging this. Is there any chance that it could get released soon?

chore(azurerm): upgrade to 3.3.0

7be2812

Signed-off-by: Toni Tauro <[email protected]>

davidkarlsen mentioned this pull request May 20, 2022

export addon_profile attributes #151

Merged

lonegunmanb requested changes Jun 7, 2022

View reviewed changes

eyenx and others added 2 commits June 15, 2022 11:10

Update variables.tf

6f7f6fd

Co-authored-by: lonegunmanb <[email protected]>

Update outputs.tf

08061c5

Co-authored-by: lonegunmanb <[email protected]>

fix(outputs.tf) sensitive values

75925c5

fix(ouptuts): sensitive tests

c6c308c

Signed-off-by: Toni Tauro <[email protected]>

eyenx requested a review from lonegunmanb June 16, 2022 08:34

lonegunmanb approved these changes Jun 16, 2022

View reviewed changes

lonegunmanb merged commit 0b56a45 into Azure:master Jun 16, 2022

eyenx deleted the chore/ugrade-azurerm-to-3.3.0 branch June 16, 2022 10:06

zioproto mentioned this pull request Jun 20, 2022

Refactor output for removed addon_profile block. Deprecate legacy dot index syntax in outputs. #188

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(azurerm): upgrade to 3.3.0 #157

chore(azurerm): upgrade to 3.3.0 #157

eyenx commented Apr 22, 2022

ghost commented Apr 22, 2022 •

edited by ghost

Loading

eyenx commented May 20, 2022

lonegunmanb commented May 30, 2022

lonegunmanb left a comment

lonegunmanb Jun 6, 2022

eyenx Jun 15, 2022

lonegunmanb Jun 6, 2022

eyenx Jun 15, 2022

lonegunmanb commented Jun 7, 2022

eyenx commented Jun 15, 2022

lonegunmanb commented Jun 15, 2022

eyenx commented Jun 15, 2022

lonegunmanb commented Jun 15, 2022

eyenx commented Jun 16, 2022

eyenx commented Jun 16, 2022

lonegunmanb commented Jun 16, 2022

davidkarlsen commented Jun 16, 2022

lonegunmanb commented Jun 16, 2022 •

edited

Loading

hairmare commented Jul 26, 2022 •

edited

Loading

chore(azurerm): upgrade to 3.3.0 #157

chore(azurerm): upgrade to 3.3.0 #157

Conversation

eyenx commented Apr 22, 2022

ghost commented Apr 22, 2022 • edited by ghost Loading

eyenx commented May 20, 2022

lonegunmanb commented May 30, 2022

lonegunmanb left a comment

Choose a reason for hiding this comment

lonegunmanb Jun 6, 2022

Choose a reason for hiding this comment

eyenx Jun 15, 2022

Choose a reason for hiding this comment

lonegunmanb Jun 6, 2022

Choose a reason for hiding this comment

eyenx Jun 15, 2022

Choose a reason for hiding this comment

lonegunmanb commented Jun 7, 2022

eyenx commented Jun 15, 2022

lonegunmanb commented Jun 15, 2022

eyenx commented Jun 15, 2022

lonegunmanb commented Jun 15, 2022

eyenx commented Jun 16, 2022

eyenx commented Jun 16, 2022

lonegunmanb commented Jun 16, 2022

davidkarlsen commented Jun 16, 2022

lonegunmanb commented Jun 16, 2022 • edited Loading

hairmare commented Jul 26, 2022 • edited Loading

ghost commented Apr 22, 2022 •

edited by ghost

Loading

lonegunmanb commented Jun 16, 2022 •

edited

Loading

hairmare commented Jul 26, 2022 •

edited

Loading