Addition of resource and data source for tenant_policies_igmp_interface_policy (DCNE-278) by shrsr · Pull Request #403 · CiscoDevNet/terraform-provider-mso

shrsr · 2025-11-27T18:11:01Z

No description provided.

akinross · 2025-11-28T14:14:10Z

mso/utils.go

 	return nil
 }

+func removePatchPayloadToContainer(payloadContainer *container.Container, op, path string) error {


why do you need a new function for this? cant you pass in nil to value in the addPatchPayloadToContainer function and check if value nil then change the payload to not include value?

I actually think it's better to have a dedicated function for remove. If we want to have both add and remove functionality in the same function it would be best to update the name of addPatchPayloadToContainer to updatePatchPayloadToContainer or similar.

I think the naming of add is not linked to the patch operation but the adding of a PATCH payload to the container. So whatever operation add/replace/remove the PATCH payload has could be handled by same function. If you think it is clearer this way should we then also add separate function for replace?

True. It's not operation related..

akinross · 2025-11-28T14:17:27Z

mso/resource_mso_tenant_policies_igmp_interface_policy.go

+	if d.HasChange("state_limit_route_map_uuid") {
+		uuid := d.Get("state_limit_route_map_uuid").(string)
+		if uuid != "" {
+			err := addPatchPayloadToContainer(payloadCont, "replace", fmt.Sprintf("%s/stateLimitRouteMapRef", updatePath), uuid)
+			if err != nil {
+				return err
+			}
+		} else {
+			err := removePatchPayloadToContainer(payloadCont, "remove", fmt.Sprintf("%s/stateLimitRouteMapRef", updatePath))
+			if err != nil {
+				return err
+			}
+		}
+	}


does this work properly in older sdk version? i thought there was not a clear way to distinguish between not provided and empty string?

Yes, it works well.
If not provided the ok in ok, val:=d.GetOk("some_string") won't have true and d.HasChange("some_string") won't be true either.

Well it works that a change is detected on "" or commented out. However both delete the configuration, because we cannot differentiate between empty string or not provided configuration. So let me rephrase my question is it the intend that commenting out the config or providing null explicitly also leads to destroy operation to be invoked?

yes, both removing/commenting the attribute and its value or providing "" will invoke the remove operation. I actually removed the redundant code in the latest change.

akinross · 2025-11-28T14:17:40Z

mso/resource_mso_tenant_policies_igmp_interface_policy.go

+			}
+		}
+	}
+	log.Printf("HERE %v", payloadCont)


akinross · 2025-12-04T09:45:33Z

examples/tenant_policies_igmp_interface_policy/main.tf

+}
+
+resource "mso_tenant_policies_route_map_policy_multicast" "state_limit" {
+  template_id = mso_template.template_tenant.id


Suggested change

template_id = mso_template.template_tenant.id

template_id = mso_template.tenant_template.id

akinross · 2025-12-04T09:54:30Z

examples/tenant_policies_igmp_interface_policy/main.tf

+# tenant policies igmp interface policy example
+
+resource "mso_tenant_policies_igmp_interface_policy" "igmp_policy" {
+  template_id                  = mso_template.template_tenant.id


Suggested change

template_id = mso_template.template_tenant.id

template_id = mso_template.tenant_template.id

samiib

LGTM

akinross

LGTM

gmicol

LGTM

akinross · 2025-12-18T10:26:47Z

mso/resource_mso_tenant_policies_igmp_interface_policy.go

+		return err
+	}
+
+	d.SetId(fmt.Sprintf("templateId/%s/IGMPInterfacePolicy/%s", templateId, d.Get("name").(string)))


Is there a reason to capitalise IGMP on the setID when the list equivalent is igmpInterfacePolicies? What is the rule we follow for setting this?

I don't remember why I went with capitalizing for the first resource (IPSLA) I created but this is the rule we've been following for all the the NDO TF Ids

akinross · 2025-12-18T10:28:59Z

mso/resource_mso_tenant_policies_igmp_interface_policy.go

+		d.Set("report_link_local_groups", reportLinkLocal)
+	}
+
+	d.Set("igmp_version", models.StripQuotes(response.S("igmpQuerierVersion").String()))


why is there no response.Exists() for all of the attributes to be set? is this because some would always exist?

I missed it here

akinross · 2025-12-18T10:33:04Z

mso/resource_mso_tenant_policies_igmp_interface_policy.go

+		enableV3Asm, _ := response.S("enableV3Asm").Data().(bool)
+		d.Set("version3_asm", enableV3Asm)


is bool casting not working directly?, what is the _ for, an error? Is there a case it can error?

the _ should actually be 'ok' indicating whether the type assertion succeeded. However, we're using response.Exists() so it should be fine leaving it blank. I don't think we need it the assertion check because API always returns the same type unless they were to change true to enable at some point. I modified to include ok here.

akinross

LGTM

samiib

LGTM

gmicol

LGTM

lhercot

LGTM

lhercot

See issue in Go Release check: https://github.com/CiscoDevNet/terraform-provider-mso/actions/runs/20343107477/job/60651101226

Seems we need to reduce the test value to be under a int32

shrsr · 2026-01-20T17:24:00Z

The build likely failed because Go Releaser was compiling for 32-bit systems (target=freebsd_386_sse2) where the maximum integer value is way less than what's specified in the upper limit (ValidateFunc: validation.IntBetween(1, 4294967295)) which is too large for this system to handle.

…es_igmp_interface_policy

…s_igmp_interface_policy

…ons used in resource_mso_tenant_policies_igmp_interface_policy

…ies_igmp_interface_policy

…enant_policies_igmp_interface_policy

…_interface_policy

…nterface_policy to make type assertion checks

…se validateUint32Range()

anvitha-jain

LGTM

akinross

LGTM

gmicol

LGTM

sajagana

LGTM!

samiib

LGTM

lhercot

LGTM

shrsr self-assigned this Nov 27, 2025

shrsr requested review from akinross, anvitha-jain, gmicol, lhercot, sajagana and samiib as code owners November 27, 2025 18:11

shrsr linked an issue Nov 27, 2025 that may be closed by this pull request

Create a resource and data source for "IGMP Interface Policy" object under Tenant Policies (DCNE-278) #325

Open

github-actions bot changed the title ~~Addition of resource and data source for tenant_policies_igmp_interface_policy~~ Addition of resource and data source for tenant_policies_igmp_interface_policy (DCNE-278) Nov 27, 2025

akinross requested changes Nov 28, 2025

View reviewed changes

shrsr requested a review from akinross December 3, 2025 17:57

akinross requested changes Dec 4, 2025

View reviewed changes

shrsr requested a review from akinross December 4, 2025 16:39

samiib previously approved these changes Dec 5, 2025

View reviewed changes

akinross previously approved these changes Dec 10, 2025

View reviewed changes

gmicol previously approved these changes Dec 10, 2025

View reviewed changes

shrsr dismissed stale reviews from gmicol, akinross, and samiib via 0d2496a December 15, 2025 18:47

shrsr force-pushed the igmp branch from cf54b07 to 0d2496a Compare December 15, 2025 18:47

shrsr requested review from akinross, gmicol and samiib December 16, 2025 03:29

akinross requested changes Dec 18, 2025

View reviewed changes

shrsr requested a review from akinross December 18, 2025 16:05

akinross previously approved these changes Dec 18, 2025

View reviewed changes

samiib previously approved these changes Jan 6, 2026

View reviewed changes

gmicol previously approved these changes Jan 6, 2026

View reviewed changes

lhercot approved these changes Jan 17, 2026

View reviewed changes

lhercot requested changes Jan 17, 2026

View reviewed changes

shrsr dismissed stale reviews from gmicol, samiib, and akinross via d17898f January 20, 2026 17:35

shrsr added 8 commits January 20, 2026 12:38

[minor_change] Addition of resource and data source for tenant_polici…

7f0b758

…es_igmp_interface_policy

[ignore] Removed debug log statement from resource_mso_tenant_policie…

4bb71db

…s_igmp_interface_policy

[ignore] Modified addPatchPayloadToContainer to handle remove operati…

ebd9342

…ons used in resource_mso_tenant_policies_igmp_interface_policy

[ignore] Removed redundant code for remove operations in tenant_polic…

12b08e8

…ies_igmp_interface_policy

[ignore] Reverted code to invoke remove op on certain conditions in t…

1ac5207

…enant_policies_igmp_interface_policy

[ignore] Removal of default values from docs for tenant_policies_igmp…

d0856a2

…_interface_policy

[ignore] Modified setting data in resource_mso_tenant_policies_igmp_i…

f3fee2c

…nterface_policy to make type assertion checks

[ignore] Made resource_mso_tenant_policies_igmp_interface_policy to u…

5c148ed

…se validateUint32Range()

shrsr force-pushed the igmp branch from d17898f to 5c148ed Compare January 20, 2026 17:38

shrsr requested review from akinross, gmicol, lhercot and samiib January 20, 2026 17:38

anvitha-jain approved these changes Jan 20, 2026

View reviewed changes

akinross approved these changes Jan 21, 2026

View reviewed changes

gmicol approved these changes Jan 21, 2026

View reviewed changes

sajagana approved these changes Jan 27, 2026

View reviewed changes

samiib approved these changes Feb 2, 2026

View reviewed changes

lhercot approved these changes Feb 23, 2026

View reviewed changes

	template_id = mso_template.template_tenant.id
	template_id = mso_template.tenant_template.id

		enableV3Asm, _ := response.S("enableV3Asm").Data().(bool)
		d.Set("version3_asm", enableV3Asm)

Conversation

shrsr commented Nov 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samiib left a comment

Choose a reason for hiding this comment

Uh oh!

akinross left a comment

Choose a reason for hiding this comment

Uh oh!

gmicol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shrsr Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinross Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinross left a comment

Choose a reason for hiding this comment

Uh oh!

samiib left a comment

Choose a reason for hiding this comment

Uh oh!

gmicol left a comment

Choose a reason for hiding this comment

Uh oh!

lhercot left a comment

Choose a reason for hiding this comment

Uh oh!

lhercot left a comment

Choose a reason for hiding this comment

Uh oh!

shrsr commented Jan 20, 2026

Uh oh!

anvitha-jain left a comment

Choose a reason for hiding this comment

Uh oh!

akinross left a comment

Choose a reason for hiding this comment

Uh oh!

gmicol left a comment

Choose a reason for hiding this comment

Uh oh!

sajagana left a comment

Choose a reason for hiding this comment

Uh oh!

samiib left a comment

shrsr Dec 18, 2025 •

edited

Loading

akinross Dec 18, 2025 •

edited

Loading