Skip to content
This repository has been archived by the owner on Feb 14, 2024. It is now read-only.

refactor(module/appgw): Refactor module Application Gateway and adjust examples #345

Merged
merged 94 commits into from
Dec 12, 2023

Conversation

sebastianczech
Copy link
Contributor

@sebastianczech sebastianczech commented Oct 13, 2023

Description

PR delivers changes for Application Gateway:

  • README.md adjusted to new style with additional header file
  • minimum Terraform version set to 1.3
  • refactor variables:
    • descriptions
    • types with optionals
    • default values
    • validations
  • simplify main.tf by removing not required try
  • remove name generation logic from the module

Motivation and Context

#307 , #325

How Has This Been Tested?

I checked that module using example included in PR.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

@sebastianczech sebastianczech changed the title feat(modules/appgw): Refactor module Application Gateway and adjust examples refactor(modules/appgw): Refactor module Application Gateway and adjust examples Oct 16, 2023
@sebastianczech sebastianczech changed the title refactor(modules/appgw): Refactor module Application Gateway and adjust examples refactor(module/appgw): Refactor module Application Gateway and adjust examples Oct 17, 2023
@FoSix FoSix added the refactor label Oct 19, 2023
@sebastianczech sebastianczech marked this pull request as ready for review October 19, 2023 12:54
@sebastianczech sebastianczech requested a review from a team as a code owner October 19, 2023 12:54
@sebastianczech
Copy link
Contributor Author

sebastianczech commented Oct 19, 2023

/help

ChatOPS built in help:

Currently supported commands include:

  • /sca - run all SCA tests via pre-commit
  • /validate - run terraform validate
  • /plan - plan the infrastructure (only examples)
  • /apply - deploy the infrastructure and destroy afterwards (only examples)
  • /idempotence - test idempotence: deploy, plan and destroy afterwards (only examples).

The 1st command does not take arguments, the remaining take two:

  • paths - a space delimitied list of module paths
  • tf_version - (optional, defaults to the latest available) a space delimited list of Terraform versions to test the infrastrucure against.

Examples:

# run idempotence tests on listed modules with Terraform versions: 1.2 (latest patch available), 1.4 (latest patch available), 1.5.4.
/idempotence paths="examples/common_vmseries examples/panorama_standalone" tf_version="1.2 1.4 1.5.4"
# run validation tests with the latest available Terraform version on listed modules.
/validate paths="modules/vmseries modules/vnet examples/dedicated_vmseries"

@sebastianczech
Copy link
Contributor Author

sebastianczech commented Oct 20, 2023

/plan paths="examples/common_vmseries examples/common_vmseries_and_autoscale examples/dedicated_vmseries examples/dedicated_vmseries_and_autoscale examples/virtual_network_gateway examples/appgw"

Testing job ID: 6584100272
Job result: FAILURE

@acelebanski
Copy link
Contributor

Deploying Application Gateway in WAFv2 SKU will fail. Adding this block to module's main.tf should solve the issue:

  waf_configuration {
    enabled          = var.waf_enabled
    firewall_mode    = "Detection"
    rule_set_type    = "OWASP"
    rule_set_version = "3.2"
  }

@sebastianczech
Copy link
Contributor Author

Deploying Application Gateway in WAFv2 SKU will fail. Adding this block to module's main.tf should solve the issue:

  waf_configuration {
    enabled          = var.waf_enabled
    firewall_mode    = "Detection"
    rule_set_type    = "OWASP"
    rule_set_version = "3.2"
  }

Yes, thank you for feedback. I added support for WAF in Application Gateway: f9c1b6f

Copy link
Contributor

@FoSix FoSix left a comment

Choose a reason for hiding this comment

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

just some small comments

modules/appgw/.header.md Outdated Show resolved Hide resolved
modules/appgw/.header.md Show resolved Hide resolved
modules/appgw/main.tf Outdated Show resolved Hide resolved
modules/appgw/variables.tf Outdated Show resolved Hide resolved
modules/appgw/variables.tf Outdated Show resolved Hide resolved
modules/appgw/variables.tf Outdated Show resolved Hide resolved
modules/appgw/variables.tf Show resolved Hide resolved
modules/appgw/variables.tf Outdated Show resolved Hide resolved
modules/appgw/variables.tf Show resolved Hide resolved
modules/appgw/main.tf Show resolved Hide resolved
@sebastianczech sebastianczech merged commit f24fbea into 307-refactor-modules Dec 12, 2023
@sebastianczech sebastianczech deleted the 325-appgw branch December 12, 2023 10:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants