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

Documentation fixes due to missing variable attributes #356

Merged
merged 6 commits into from
May 9, 2022
Merged

Documentation fixes due to missing variable attributes #356

merged 6 commits into from
May 9, 2022

Conversation

jonclyde
Copy link
Contributor

@jonclyde jonclyde commented May 2, 2022

Overview/Summary

Changes to the documentation due to the addition of new variables in the Azure Landing Zone module that were causing terraform to fail at apply stage.

This was the error when following the deploy connectivity resources with custom settings article:
image

Resolved by adding advanced_vpn_settings as per default values in variables.tf of the module.

This was the error when following the deploy management resources with custom settings article:

image

Resolved by adding enabled_defender_for_containers with a value of true.

This PR fixes/adds/changes/removes

  1. Error for missing "advanced_vpn_settings" variable attribute after following the article for deploy connectivity resources with custom settings and running terraform apply.
  2. Error for missing "enable_defender_for_containers" variable attribute after following the article for deploy management resources with custom settings and running terraform apply.
  3. Fixes Bug Report #366

Breaking Changes

  1. None

Testing Evidence

Apply works for deploying connectivity resources with custom settings:
image

Apply works for deploying management resources with custom settings:
image

As part of this Pull Request I have

  • Checked for duplicate Pull Requests
  • Associated it with relevant issues, for tracking and closure.
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Performed testing and provided evidence.
  • Updated relevant and associated documentation.
  • [N\A] Updated the "What's New?" wiki page (located in the Enterprise-Scale repo in the directory: /docs/wiki/whats-new.md)

jonclyde added 2 commits May 2, 2022 19:03
Following this page led to a terraform error for a missing "advanced_vpn_settings" attribute, which is a recently added attribute to the variables.tf of the Azure Landing Zone module. 

Updated settings.connectivity.tf example in this documentation page to include advanced_vpn_settings, as per the default values specified in variables.tf of the module. This resolves the TF error.
Following this page led to a terraform error for a missing "enable_defender_for_containers" attribute. 

Updated settings.management.tf example in this documentation page to include enable_defender_for_containers , with a value of true. This resolves the TF error.
@ghost
Copy link

ghost commented May 2, 2022

CLA assistant check
All CLA requirements met.

@krowlandson
Copy link
Contributor

This looks good to me @jonclyde. Thank you for identifying this and raising the PR to fix.

@matt-FFFFFF would you mind giving this the once over to ensure we didn't miss any other Wiki examples and then we can merge.

Thank you!

1 similar comment
@krowlandson
Copy link
Contributor

This looks good to me @jonclyde. Thank you for identifying this and raising the PR to fix.

@matt-FFFFFF would you mind giving this the once over to ensure we didn't miss any other Wiki examples and then we can merge.

Thank you!

@krowlandson krowlandson requested review from matt-FFFFFF May 4, 2022 04:37
@matt-FFFFFF
Copy link
Member

Thanks - I will take a look next week

@matt-FFFFFF matt-FFFFFF self-assigned this May 5, 2022
@SteveBurkettNZ
Copy link
Contributor

SteveBurkettNZ commented May 6, 2022

@matt-FFFFFF: The following lines should also be removed from the security_center settings section since they've been replaced by this new enable_defender_for_containers variable:

enable_defender_for_acr = true
enable_defender_for_kubernetes = true

@jtracey93 jtracey93 mentioned this pull request May 6, 2022
@matt-FFFFFF
Copy link
Member

/azp run unit

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matt-FFFFFF matt-FFFFFF merged commit 9a1b66e into Azure:main May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug Report
4 participants