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

[Bug]: Firewall rules DELETED when name or labels in hcloud_firewall resource are changed #931

Open
borisceranic opened this issue May 22, 2024 · 6 comments
Assignees

Comments

@borisceranic
Copy link

What happened?

While working with hcloud_firewall resource, I noticed that ALL RULES are silently deleted by Terraform hcloud provider when Terraform wants to update the firewall in-place in cases when any argument (other than rule) changes.

Consider this example:

resource "hcloud_firewall" "test" {
  name = "firewall_1"
  apply_to {
    label_selector = "test/label-1"
  }
  rule {
    description = "allow all traffic from particular IPs"
    direction   = "in"
    protocol    = "tcp"
    port        = "1-65535"
    source_ips  = [
      "1.2.3.4",
      "1.3.4.5",
    ]
  }
}

Terraform creates the resource, and it appears Just Fine (tm) in the Hetzner Cloud Console:

Terraform will perform the following actions:

  # module.this.hcloud_firewall.test will be created
  + resource "hcloud_firewall" "test" {
      + id     = (known after apply)
      + labels = (known after apply)
      + name   = "firewall_1"

      + apply_to {
          + label_selector = "test/label-1"
          + server         = (known after apply)
        }

      + rule {
          + description     = "allow all traffic from particular IPs"
          + destination_ips = []
          + direction       = "in"
          + port            = "1-65535"
          + protocol        = "tcp"
          + source_ips      = [
              + "1.2.3.4/32",
              + "1.3.4.5/32",
            ]
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.
Screenshot 2024-05-22 at 09 06 41

Then, if I update only name argument, Terraform plan shows something that appears perfectly normal and expected:

Terraform will perform the following actions:

  # module.this.hcloud_firewall.test will be updated in-place
  ~ resource "hcloud_firewall" "test" {
        id     = "1404982"
      ~ name   = "firewall_1" -> "firewall_test"
        # (1 unchanged attribute hidden)

        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Similarly, here's a perfectly normal-looking plan output when changing apply_to.label_selector argument:

Terraform will perform the following actions:

  # module.this.hcloud_firewall.test will be updated in-place
  ~ resource "hcloud_firewall" "test" {
        id     = "1404986"
        name   = "firewall_1"
        # (1 unchanged attribute hidden)

      - apply_to {
          - label_selector = "test/label-1" -> null
          - server         = 0 -> null
        }
      + apply_to {
          + label_selector = "test/label-test"
          + server         = (known after apply)
        }

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Meanwhile, looking at the Cloud Console, the end result is actually a removal of ALL RULES:

Screenshot 2024-05-22 at 09 07 12

The following re-run of terraform plan will first refresh all resources, it will pick up the missing rules, and then it will helpfully offer to re-create them on the next apply:

Terraform will perform the following actions:

  # module.this.hcloud_firewall.test will be updated in-place
  ~ resource "hcloud_firewall" "test" {
        id     = "1404986"
        name   = "firewall_1"
        # (1 unchanged attribute hidden)

      + rule {
          + description     = "allow all traffic from particular IPs test"
          + destination_ips = []
          + direction       = "in"
          + port            = "1-65535"
          + protocol        = "tcp"
          + source_ips      = [
              + "1.2.3.4/32",
              + "1.3.4.5/32",
            ]
        }

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

What did you expect to happen?

When changing name, labels and apply_to arguments, I expected rules specified via the rule {} block to remain set in the firewall.

Please provide a minimal working example

resource "hcloud_firewall" "test" {
  name = "firewall_1"
  apply_to {
    label_selector = "test/label-1"
  }
  rule {
    description = "allow all traffic from particular IPs"
    direction   = "in"
    protocol    = "tcp"
    port        = "1-65535"
    source_ips  = [
      "1.2.3.4",
      "1.3.4.5",
    ]
  }
}
@borisceranic borisceranic changed the title [Bug]: Firewall rules DELETED when non-rules section of the hcloud_firewall resource changes [Bug]: Firewall rules DELETED when name or labels in hcloud_firewall resource are changed May 22, 2024
@apricote apricote self-assigned this May 22, 2024
@apricote
Copy link
Member

Hey @borisceranic,

looks like this bug was introduced in #874, first released in 1.46.0. This only happens if you omit the /32 at the end of your allowed IPs. If you specify them, no rules are being removed:

resource "hcloud_firewall" "test" {
  name = "firewall_1"
  apply_to {
    label_selector = "test/label-1"
  }
  rule {
    description = "allow all traffic from particular IPs"
    direction   = "in"
    protocol    = "tcp"
    port        = "1-65535"
    source_ips  = [
      "1.2.3.4/32",
      "1.3.4.5/32",
    ]
  }
}

We will take a look at this.

@apricote
Copy link
Member

This is reproducable with this e2e test:

func TestFirewallResource_Regression931(t *testing.T) {
	var f hcloud.Firewall

	res := firewall.NewRData(t, "update-keep-rules", []firewall.RDataRule{
		{
			Direction:   "in",
			Protocol:    "tcp",
			SourceIPs:   []string{"192.0.2.250"},
			Port:        "80",
			Description: "allow http in",
		},
	}, nil)

	updated := &firewall.RData{
		Name:    "update-keep-rules-changed",
		Rules:   res.Rules,
		ApplyTo: res.ApplyTo,
		Labels:  res.Labels,
	}
	updated.SetRName(res.RName())

	tmplMan := testtemplate.Manager{}
	resource.Test(t, resource.TestCase{
		PreCheck:                 teste2e.PreCheck(t),
		ProtoV6ProviderFactories: teste2e.ProtoV6ProviderFactories(),
		CheckDestroy:             testsupport.CheckResourcesDestroyed(firewall.ResourceType, firewall.ByID(t, &f)),
		Steps: []resource.TestStep{
			{
				Config: tmplMan.Render(t, "testdata/r/hcloud_firewall", res),
				Check:  testsupport.CheckResourceExists(res.TFID(), firewall.ByID(t, &f)),
			},
			{
				// Update something other than the rules
				Config: tmplMan.Render(t, "testdata/r/hcloud_firewall", updated),
			},
		},
	})
}

@borisceranic
Copy link
Author

borisceranic commented May 22, 2024

Hey @apricote ,

looks like this bug was introduced in #874, first released in 1.46.0. This only happens if you omit the /32 at the end of your allowed IPs. If you specify them, no rules are being removed:

Thanks, that's a viable workaround, I'll implement it as a stop-gap solution to prevent possible issues, until the provider is fixed.

By the way: are you saying that if I stick with the provider 1.45.0 or older (for the time being) that the issue would not happen?

@apricote
Copy link
Member

By the way: are you saying that if I stick with the provider 1.45.0 or older (for the time being) that the issue would not happen?

In theory yes, but before 1.46.0 you got an error if you specified the IP directly without /32, so you would still have to add that to your configuration.

@apricote
Copy link
Member

Looks like we are running into the same bug as described here: #468

Copy link

This issue has been marked as stale because it has not had recent activity. The bot will close the issue if no further action occurs.

@github-actions github-actions bot added the stale label Aug 20, 2024
@jooola jooola added pinned and removed stale labels Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants