[bugfix] fix child classes to be queried only when apic versions allows (DCNE-589) by akinross · Pull Request #1407 · CiscoDevNet/terraform-provider-aci

akinross · 2025-11-25T20:05:51Z

No description provided.

shrsr

LGTM

samiib · 2025-12-02T12:34:31Z

gen/templates/resource_test.go.tmpl

 					resource.TestCheckResourceAttrSet("aci_{{$.resourceName}}.test", "{{$key}}"),
 			{{- end}}
 			{{- range $key, $value := $.children}}
+			// foo


Suggested change

// foo

gmicol

LGTM

gmicol

LGTM

samiib

LGTM

shrsr

LGTM

shrsr

LGTM

shrsr

Needs rebase

shrsr

LGTM

anvitha-jain

LGTM

samiib

LGTM

gmicol

LGTM

lhercot · 2026-01-17T02:59:02Z

gen/templates/provider.go.tmpl

 }
+
+func getVersionAPIC(ctx context.Context, diags *diag.Diagnostics, client *client.Client) string {
+	requestData := DoRestRequest(ctx, diags, client, fmt.Sprintf("/api/node/class/topSystem.json"), "GET", nil)


We should add a filter on the query for the role == controller to reduce size as this would query the whole set of switches which could be hundreds.

good one, adjusted the query, which also simplifies the loop

lhercot · 2026-01-17T03:01:22Z

gen/templates/provider.go.tmpl

+	}
+
+	if requestData.Search("imdata").Search("topSystem").Data() != nil {
+		attributes := requestData.Search("imdata").Search("topSystem").Search("attributes").Data().([]interface{})


Does this merge attributes with same value but not with different values?

I am not sure what you mean by this comment but I did spot an issue with the version appending. The duplicates should be removed for the length check

lhercot · 2026-01-17T03:01:40Z

gen/templates/provider.go.tmpl

+		)
+	} else {
+		diags.AddError(
+			"Data for topSysytem class could not be retrieved",


Suggested change

"Data for topSysytem class could not be retrieved",

"Data for topSystem class could not be retrieved",

will change

lhercot · 2026-01-17T03:08:15Z

internal/provider/versions.go

+
+package provider
+
+func classVersions() map[string]string {


Any reason to use function for this vs a constant?

a constant is not possible because a map cannot be a constant in go, will leverage a package-level variable instead of a function

lhercot · 2026-01-17T03:11:24Z

internal/provider/utils.go

@@ -17,6 +18,19 @@ import (
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 )

+func getChildClassesForGetRequest(childClasses []string) []string {
+	classVersions := classVersions()


I think this should be defined as a constant and not a function so we avoid re-addressing the list everytime.

a constant is not possible because a map cannot be a constant in go, will leverage a package-level variable.

lhercot · 2026-01-17T03:13:13Z

internal/provider/resource_aci_access_port_block.go


 func getAndSetInfraPortBlkAttributes(ctx context.Context, diags *diag.Diagnostics, client *client.Client, data *InfraPortBlkResourceModel) {
-	requestData := DoRestRequest(ctx, diags, client, fmt.Sprintf("api/mo/%s.json?rsp-subtree=full&rsp-subtree-class=%s", data.Id.ValueString(), "infraPortBlk,infraRsAccBndlSubgrp,tagAnnotation,tagTag,tagAnnotation,tagTag"), "GET", nil)
+	childClasses := getChildClassesForGetRequest([]string{"infraRsAccBndlSubgrp", "tagAnnotation", "tagTag"})


Where did infraPortBlk go?

infraPortBlk is the the class being requested in the resource via the DN set in data.Id.ValueString(), so not sure why this would be needed to be set in the rsp-subtree-class ?

lhercot · 2026-01-17T03:14:12Z

internal/provider/resource_aci_access_port_selector.go


 func getAndSetInfraHPortSAttributes(ctx context.Context, diags *diag.Diagnostics, client *client.Client, data *InfraHPortSResourceModel) {
-	requestData := DoRestRequest(ctx, diags, client, fmt.Sprintf("api/mo/%s.json?rsp-subtree=full&rsp-subtree-class=%s", data.Id.ValueString(), "infraHPortS,infraRsAccBaseGrp,tagAnnotation,tagTag,tagAnnotation,tagTag"), "GET", nil)
+	childClasses := getChildClassesForGetRequest([]string{"infraRsAccBaseGrp", "tagAnnotation", "tagTag"})


Where did infraHPortS go?

infraHPortS is the the class being requested in the resource via the DN set in data.Id.ValueString(), so not sure why this would be needed to be set in the rsp-subtree-class ?

lhercot · 2026-01-17T03:16:28Z

gen/definitions/classes/commPol.yaml

@@ -1,4 +1,4 @@
-class_version: 6.0(2h)-6.0(8g),6.1(3f)-


shouldn't it be defined for both?

This was done only for testing, the commPol class itself is supported but there was some behaviour of the class which made us skip it in those versions of testing. I did not want to change this behaviour but I needed a way to mention deprecated class like the case of fvRsBDToRelayP. So the class_version is now use for overall override of the class version when not in meta. The class_version_tests is only applied to the tests.

…of possible response by filter the topSystem request

gmicol

LGTM

shrsr

LGTM

anvitha-jain

LGTM

sajagana

LGTM!

akinross requested review from anvitha-jain, gmicol, lhercot, sajagana, samiib and shrsr as code owners November 25, 2025 20:05

akinross added the jira-sync Sync this issue to Jira label Nov 25, 2025

akinross force-pushed the bug_version_check_children branch from dc6fb13 to a410de7 Compare November 25, 2025 20:20

shrsr previously approved these changes Nov 26, 2025

View reviewed changes

shrsr added jira-sync Sync this issue to Jira and removed jira-sync Sync this issue to Jira labels Nov 26, 2025

github-actions bot changed the title ~~[bugfix] fix child classes to be queried only when apic versions allows~~ [bugfix] fix child classes to be queried only when apic versions allows (DCNE-589) Nov 26, 2025

samiib requested changes Dec 2, 2025

View reviewed changes

akinross dismissed shrsr’s stale review via b6fb1ff December 2, 2025 13:27

akinross requested review from samiib and shrsr December 2, 2025 13:30

gmicol previously approved these changes Dec 2, 2025

View reviewed changes

akinross dismissed gmicol’s stale review via 1622edf December 2, 2025 15:45

akinross force-pushed the bug_version_check_children branch from b6fb1ff to 1622edf Compare December 2, 2025 15:45

akinross requested a review from gmicol December 2, 2025 15:45

gmicol previously approved these changes Dec 2, 2025

View reviewed changes

samiib previously approved these changes Dec 2, 2025

View reviewed changes

shrsr previously approved these changes Dec 2, 2025

View reviewed changes

akinross dismissed stale reviews from shrsr, samiib, and gmicol via 43b7294 December 3, 2025 06:56

akinross requested review from gmicol, samiib and shrsr December 3, 2025 06:57

shrsr previously approved these changes Dec 3, 2025

View reviewed changes

shrsr self-requested a review December 18, 2025 15:25

shrsr requested changes Dec 18, 2025

View reviewed changes

akinross requested a review from shrsr January 12, 2026 08:05

[bugfix] fix child classes to be queried only when apic versions allows

0f40f27

akinross dismissed stale reviews from gmicol, samiib, and sajagana via 0f40f27 January 12, 2026 08:48

akinross force-pushed the bug_version_check_children branch from cd1c8d8 to 0f40f27 Compare January 12, 2026 08:48

akinross requested review from gmicol, sajagana and samiib January 12, 2026 08:49

shrsr previously approved these changes Jan 12, 2026

View reviewed changes

anvitha-jain previously approved these changes Jan 13, 2026

View reviewed changes

samiib previously approved these changes Jan 14, 2026

View reviewed changes

gmicol previously approved these changes Jan 14, 2026

View reviewed changes

lhercot requested changes Jan 17, 2026

View reviewed changes

[ignore] make classVersion a package level variable, reduce the size …

6a21e40

…of possible response by filter the topSystem request

akinross dismissed stale reviews from gmicol, samiib, anvitha-jain, and shrsr via 6a21e40 January 19, 2026 10:39

akinross requested review from anvitha-jain, gmicol, lhercot, samiib and shrsr January 19, 2026 10:51

gmicol approved these changes Jan 21, 2026

View reviewed changes

shrsr approved these changes Jan 21, 2026

View reviewed changes

anvitha-jain approved these changes Jan 26, 2026

View reviewed changes

sajagana approved these changes Jan 27, 2026

View reviewed changes

	"Data for topSysytem class could not be retrieved",
	"Data for topSystem class could not be retrieved",

Comments

Conversation

akinross commented Nov 25, 2025

Uh oh!

shrsr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmicol 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!

samiib left a comment

Choose a reason for hiding this comment

Uh oh!

shrsr left a comment

Choose a reason for hiding this comment

Uh oh!

shrsr left a comment

Choose a reason for hiding this comment

Uh oh!

shrsr left a comment

Choose a reason for hiding this comment

Uh oh!

shrsr left a comment

Choose a reason for hiding this comment

Uh oh!

anvitha-jain 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!

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!

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!

gmicol left a comment

Choose a reason for hiding this comment

Uh oh!

shrsr left a comment

Choose a reason for hiding this comment

Uh oh!

anvitha-jain left a comment

Choose a reason for hiding this comment

Uh oh!