CheckPointSW / terraform-provider-checkpoint

Terraform provider for Check Point
https://www.terraform.io/docs/providers/checkpoint/
Mozilla Public License 2.0
27 stars 38 forks source link

Default settings are not correct in checkpoint_management_access_rule #90

Closed harhan closed 2 years ago

harhan commented 2 years ago

When applying the HCL from the example in the documentation the subsequent plans propose changes to the applied settings. I've isolated this code/rule, that's why the position is changed.

resource "checkpoint_management_access_rule" "rule3" {
  layer = "cloudPolicy Network"
  #position = { below = checkpoint_management_access_rule.rule2.name }
  position           = { top = "top" }
  name               = "test3"
  action             = "Accept"
  source             = ["DMZNet", "DMZZone", "WirelessZone"]
  enabled            = true
  destination        = ["InternalNet", "CPDShield"]
  destination_negate = true
}

Results in plans:

  # checkpoint_management_access_rule.main will be updated in-place
  ~ resource "checkpoint_management_access_rule" "rule3" {
      ~ action_settings    = {
          - "enable_identity_captive_portal" = "false" -> null
        }
        id                 = "be91e1f6-b06d-41de-9cea-7e7ef0853d34"
        name               = "test3"
      ~ track              = {
          - "accounting"              = "false" -> null
          - "alert"                   = "none" -> null
          - "enable_firewall_session" = "false" -> null
          - "per_connection"          = "false" -> null
          - "per_session"             = "false" -> null
          - "type"                    = "None" -> null
        }
        # (19 unchanged attributes hidden)
    }

One should expect this code in the documentation is tested?

chkp-alonshev commented 2 years ago

hi @harhan, Thank you for this issue, the documentations will change in the next version. note that you need to set the default value as shown in the plan. meaning you need to set track, track->accounting, and etc. We will add the defaults to the documentations examples.

harhan commented 2 years ago

I rather see you improve the provider by setting the correct default values on optional properties. There should be no need to set these values in HCL if they are not needed. We need Check Point to make this provider developer friendly, as I requested in #88. This is going the opposite direction.

chkp-alonshev commented 2 years ago

It has upsides and downsides, not asking the user to set the default values results in the state not saving the default values, which leads to the provider's not seeing the changes in the default value as a change in plan, and that is a big downside. We are trying to make the provider as friendly as possible, but sometimes we are forced to choose proper functioning over friendliness.

harhan commented 2 years ago

Every provider I use with cloud native, for instance azurerm, uses this approach. I can inspect the state and see the default values there. If I'm not happy with what the provider has selected as default I can change it in HCL.

chkp-alonshev commented 2 years ago

No problem, We will support this in the next terraform version.

chkp-alonshev commented 2 years ago

Hi @harhan , This issue is fixed on the new provider's version, which is available now.

Thank you