pass layer & id to https_rule update call

[phkrl]

Currently, https rule update does not pass rule id & rule layer to api call, thus resulting in errors, since these fields are mandatory. This patch fixes issue, explicitly adding uid and layer fields to the structure passed to api.

[chkp-royl]

Hi @phkrl ,

Thanks for your PR.
I understand the issue and I have few comments:

1. API payload should get name or uid fields and currently both of them can be set.
2. There is no 'new-layer' field in 'set-https-rule' API request. Only 'layer' field should be used.

Thanks,
Roy

[phkrl]

Hi @chkp-royl,

1. API payload should get name or uid fields and currently both of them can be set.

This statement contradicts the issue we encountered: we explicitly set name field for each rule but still got error claiming that uid is required. It was only after this patch when we could update our existing https rules.

2. There is no 'new-layer' field in 'set-https-rule' API request. Only 'layer' field should be used.

Fixed in latest commit.

[chkp-royl]

This might happen if both rules had the same name so we can identify them only by uid.

1. Need to set layer anyway regardless if was changed, you can do it with d.Get("layer") after you handle with the uid.
2. If name was changed, just add "new-name" to the request because we don't want to have both name and uid.

[phkrl]

This might happen if both rules had the same name so we can identify them only by uid.

We have unique rule names. They are keys of map in our module.

1. Need to set layer anyway regardless if was changed, you can do it with d.Get("layer") after you handle with the uid.

I believe, this block should set layer both if it was changed and if it wasn't. Is my assumption wrong?

2. If name was changed, just add "new-name" to the request because we don't want to have both name and uid.

Do you mean, i should pass only "new-name" field, without setting "name" at all, since "uid" is passed unconditionally?

[chkp-royl]

I believe, this block should set layer both if it was changed and if it wasn't. Is my assumption wrong?

The check of HasChange is irrelevant now. We can set layer anyway just like you did with the uid.

Do you mean, i should pass only "new-name" field, without setting "name" at all, since "uid" is passed unconditionally?

yes

[phkrl]

Fixed both

[phkrl]

Added one more commit to fix destroy issue. Otherwise the error states that ignore-errors is unrecognized parameter.

[chkp-royl]

Fixed both

Looks better now.
For 'new-name' field you can use d.Get("name") instead because we don't use old value.
Move 'layer' to be right after set uid.

Added one more commit to fix destroy issue. Otherwise the error states that ignore-errors is unrecognized parameter.

Alright.

[phkrl]

Looks better now. For 'new-name' field you can use d.Get("name") instead because we don't use old value. Move 'layer' to be right after set uid.

Applied both.

[chkp-royl]

Thanks for the effort. Merged.