nsxt_policy_group bad use of Conjuction

cbreuil-orange commented 1 year ago

Describe the bug

Normally Conjunction was required only between "criteria"

Reproduction steps

Create a groups
add only one any critera
add one IP
Import this ressources

You can see a Conjunction

the Result

# nsxt_policy_group.groups:
resource "nsxt_policy_group" "groups" {
    display_name = "TEST"

    conjunction {
        operator = "OR"
    }
    criteria {
        condition {
            key         = "Name"
            member_type = "VirtualMachine"
            operator    = "EQUALS"
            value       = "TEST"
        }
    }
    criteria {
        ipaddress_expression {
            ip_addresses = [
                "1.1.1.1",
            ]
        }
    }
}

Expected behavior

The Conjuction was only expected between criteria, in the GUI interfaces you can set "AND" or "OR" after the second criteria "Membership Criteria"

the Expected

resource "nsxt_policy_group" "groups" {
    display_name = "TEST"

    criteria {
        condition {
            key         = "Name"
            member_type = "VirtualMachine"
            operator    = "EQUALS"
            value       = "TEST"
        }
    }
    criteria {
        ipaddress_expression {
            ip_addresses = [
                "1.1.1.1",
            ]
        }
    }
}

OR

resource "nsxt_policy_group" "groups" {
    display_name = "TEST"

    criteria {
        condition {
            key         = "Name"
            member_type = "VirtualMachine"
            operator    = "EQUALS"
            value       = "TEST"
        }
    }
    ipaddress_expression {
        ip_addresses = [
            "1.1.1.1",
      ]
    }
}

and if you want multiple "Membership Criteria"

resource "nsxt_policy_group" "groups" {
    display_name = "TEST"

    criteria {
        condition {
            key         = "Name"
            member_type = "VirtualMachine"
            operator    = "EQUALS"
            value       = "TEST"
        }
    conjunction {
        operator = "OR"
    }
        condition {
            key         = "Name"
            member_type = "VirtualMachine"
            operator    = "EQUALS"
            value       = "TEST2"
        }
    }
    criteria {
        ipaddress_expression {
            ip_addresses = [
                "1.1.1.1",
            ]
        }
    }
}

Additional context

If you edit the OR by AND between One Criteria "Condition" and "Ipaddress", is it impossible

│ Error: AND conjunctions must use the same types of criteria expressions, but got condition and ipaddress_expression

and the error is normal

If you use the AND operator

resource "nsxt_policy_group" "groups" {
    display_name = "TEST"
    criteria {
        ipaddress_expression {
            ip_addresses = [
                "1.1.1.2",
            ]
        }
    }
    conjunction {
        operator = "AND"
    }
    criteria {
        ipaddress_expression {
            ip_addresses = [
                "1.1.1.1",
            ]
        }
    }
}

its Dosent work

│ Error: Failed to update Group b92ffe4e-e5f9-4803-a5a1-15c4492ef3e8: The conjunction operator AND can only be used with condition expressions. (code 500302) Normal

If that

resource "nsxt_policy_group" "groups" {
    display_name = "TEST"
    criteria {
        ipaddress_expression {
            ip_addresses = [
                "1.1.1.2",
            ]
        }
    }
    conjunction {
        operator = "OR"
    }
    criteria {
        ipaddress_expression {
            ip_addresses = [
                "1.1.1.1",
            ]
        }
    }
}

its works OK but now without the Conjunction

resource "nsxt_policy_group" "groups" {
    display_name = "TEST"
    criteria {
        ipaddress_expression {
            ip_addresses = [
                "1.1.1.2",
            ]
        }
    }
    criteria {
        ipaddress_expression {
            ip_addresses = [
                "1.1.1.1",
            ]
        }
    }
}

│ Error: Missing conjunction for criteria

Yes i know i can set the second ip and the same line but its for the demonstration

I dont want a default value to OR, but a good separation between "membership critera" who need OR or AND Operator and the other Critera

annakhm commented 1 year ago

Hi @cbreuil-orange, sorry for delay in response. The OR conjunction you see comes from NSX API. If you run a GET API call on the example you have provided, you'll see the OR conjunction there:

`{

"expression": [
    {
        "member_type": "VirtualMachine",
        "key": "Tag",
        "operator": "EQUALS",
        "scope_operator": "EQUALS",
        "value": "|orange",
        "resource_type": "Condition",
        "id": "d9489942-8b6a-4e89-9a6c-daacd89773b6",
        "path": "/infra/domains/default/groups/test/condition-expressions/d9489942-8b6a-4e89-9a6c-daacd89773b6",
        "relative_path": "d9489942-8b6a-4e89-9a6c-daacd89773b6",
        "parent_path": "/infra/domains/default/groups/test",
        "remote_path": "",
        "marked_for_delete": false,
        "overridden": false,
        "_protection": "NOT_PROTECTED"
    },
    {
        "conjunction_operator": "OR",
        "resource_type": "ConjunctionOperator",
        "id": "803aa30d-2aa6-4572-aaf5-028191c063ae",
        "path": "/infra/domains/default/groups/test/conjunction-expressions/803aa30d-2aa6-4572-aaf5-028191c063ae",
        "relative_path": "803aa30d-2aa6-4572-aaf5-028191c063ae",
        "parent_path": "/infra/domains/default/groups/test",
        "remote_path": "",
        "marked_for_delete": false,
        "overridden": false,
        "_protection": "NOT_PROTECTED"
    },
    {
        "ip_addresses": [
            "12.2.2.4"
        ],
        "resource_type": "IPAddressExpression",
        "id": "1f0c0542-8685-44b4-bc6d-39703ce972d6",
        "path": "/infra/domains/default/groups/test/ip-address-expressions/1f0c0542-8685-44b4-bc6d-39703ce972d6",
        "relative_path": "1f0c0542-8685-44b4-bc6d-39703ce972d6",
        "parent_path": "/infra/domains/default/groups/test",
        "remote_path": "",
        "marked_for_delete": false,
        "overridden": false,
        "_protection": "NOT_PROTECTED"
    }
],
"extended_expression": [],
"reference": false,
"resource_type": "Group",
"id": "test",
"display_name": "test",
"path": "/infra/domains/default/groups/test",
"relative_path": "test",
"parent_path": "/infra/domains/default",
"remote_path": "",
"unique_id": "3126dbcc-60b7-444e-9053-5359ca4acfe5",
"realization_id": "3126dbcc-60b7-444e-9053-5359ca4acfe5",
"owner_id": "d23b2ec9-f9e2-4a76-ac1e-d093c058074f",
"origin_site_id": "d23b2ec9-f9e2-4a76-ac1e-d093c058074f",
"marked_for_delete": false,
"overridden": false,
"_create_time": 1690241134369,
"_create_user": "admin",
"_last_modified_time": 1690241134369,
"_last_modified_user": "admin",
"_system_owned": false,
"_protection": "NOT_PROTECTED",
"_revision": 0

}`

If I understand correctly, you are asking terraform to hide the operator in cases where the only possible value for it is OR. Is that correct?

llebotlan commented 3 months ago

Hello, I think it can be a solution. But maybe consider a issue. With terraform I wanted to produce a dynamic structure with "or" and "and" operators. I face 2 difficulties:

first, I do not find a solution to complete the list of attributes (conjuction, criteria, conjuction) in the resource dynamically. You can do it in a local variable map but not in a resources. Due to a terraform langage limitation.
second, the propose structure is static, then I think it cannot evolve. Today I notice, criteria can use 2 different typeMembers : Segment and SegmentPort. I suspect the NSXT API should evolve and your current structure will be limited.

You can solve it by creating an ordered tree structure or a list of object criteria or conjuction (+parantesis operator to manage priority) to on attribute of nsxt_policy_group . That way, you can create this complex structure in a local variable. Then, when running, let's NSX-T API told you that some functionality are not supported.. it will encourage NSX to support more powerful functionality :-) Thank you for the nsxt provider. It change the way of thinking from nsx-v.

salv-orlando commented 2 months ago

@annakhm could this be - at least partially - addressed with the changes we are planning for Issue #1266 ?

vmware / terraform-provider-nsxt