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

Add new resource cloudflare_magic_firewall_ruleset #884

Merged
merged 7 commits into from
Jan 5, 2021

Conversation

cehrig
Copy link

@cehrig cehrig commented Dec 5, 2020

Opening a draft PR for visibility.

This PR implements #880 and introduces a new resource cloudflare_magic_firewall_ruleset for managing Magic Transit Firewall Rules (Magic Firewall). Magic Firewall API is piggybacking on the new Rulesets API by using

  • ruleset kind: root
  • ruleset phase: magic_transit

Docs:

  • API Docs for the Rulesets API: https://api.cloudflare.com/#rulesets-properties
  • Magic Firewall using Rulesets: https://developers.cloudflare.com/magic-transit/magic-firewall

The ruleset kind and ruleset phase for Magic Firewall Rules are abstracted by cloudflare-go and this PR cloudflare/cloudflare-go#558

Resource Layout

resource "cloudflare_magic_firewall_ruleset" "example" {
  account_id = "b66603270ac98750f6598cba38ec720b"
  name = "Sample Ruleset"
  description = "Magic Firewall Rules"

  rules = [
    {
      action = "allow"
      expression = "tcp.dstport in { 32768..65535 }"
      description = "Allow TCP Ephemeral Ports"
      enabled = "true"
    },
    {
      action = "allow"
      expression = "udp.dstport in { 32768..65535 }"
      description = "Allow UDP Ephemeral Port range"
      enabled = "true"
    },
    {
      action = "block"
      expression = "ip.len >= 0"
      description = "Block all"
      enabled = "true"
    }
  ]
}

The resource uses a list of maps with a custom validator because the order of rules is important here. @jacobbednarz, I'd be happy to discuss if there's a smarter approach once the PR is ready to be reviewed.

Acceptance Tests:

  • Create Magic Firewall Ruleset and test if exists
  • Update Ruleset name and force new ID
  • Update Ruleset description and check if ID persists
  • Create Ruleset with single rule and test if exists
  • Update Ruleset with a higher priority rule

ToDo:

@jacobbednarz
Copy link
Member

I think you're on the right track with this 👍 The ordering you're intending to do here seems like a good fit for TypeList. As you've probably found out, the TypeMap can be a little finicky to build/expand but it can be done. It's a bit of a shame we can't use something like the following (which you added for Rulesets) but I can understand the type challenges.

rule {
  # ...
}

rule {
  # ...
}

rule {
  # ...
}

I'll hold off any deep review until the cloudflare-go changes have landed to avoid any knock on changes but this is an awesome start 👏

@cehrig cehrig marked this pull request as ready for review January 4, 2021 16:39
@cehrig cehrig changed the title WIP: Add new resource cloudflare_magic_firewall_ruleset Add new resource cloudflare_magic_firewall_ruleset Jan 4, 2021
@cehrig
Copy link
Author

cehrig commented Jan 4, 2021

@patryk @jacobbednarz I'm opening this PR for review now that we have the relevant changes in upstream. Looking forward to your thoughts!

Copy link
Member

@jacobbednarz jacobbednarz left a comment

Choose a reason for hiding this comment

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

this looks 👍 to me however this is going to break CI (just a little bit more) as we don't have a great way of hooking this up to an account designed for these acceptance tests.

i don't mind too much at the moment but it is something we should take a look at fixing eventually...somehow 😆

@jacobbednarz
Copy link
Member

linking #671 as here as well as it's a similar case whereby we can't really test it without real disruption.

@jacobbednarz
Copy link
Member

integration is green

=== RUN   TestAccCloudflareMagicFirewallRulesetExists
--- PASS: TestAccCloudflareMagicFirewallRulesetExists (8.18s)
=== RUN   TestAccCloudflareMagicFirewallRulesetUpdateName
--- PASS: TestAccCloudflareMagicFirewallRulesetUpdateName (15.39s)
=== RUN   TestAccCloudflareMagicFirewallRulesetUpdateDescription
--- PASS: TestAccCloudflareMagicFirewallRulesetUpdateDescription (14.22s)
=== RUN   TestAccCloudflareMagicFirewallRulesetSingleRule
--- PASS: TestAccCloudflareMagicFirewallRulesetSingleRule (7.56s)
=== RUN   TestAccCloudflareMagicFirewallRulesetUpdateWithHigherPriority
--- PASS: TestAccCloudflareMagicFirewallRulesetUpdateWithHigherPriority (14.47s)
PASS
ok      github.com/cloudflare/terraform-provider-cloudflare/cloudflare  59.839s
?       github.com/cloudflare/terraform-provider-cloudflare/version     [no test files]

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.

2 participants