feat: Adding Route Aggregation Rule and Connection Route Aggregation Data Sources and Resources #860

srushti-patl · 2025-02-18T22:49:11Z

fabric_route_aggregation_rule Data Sources and Resource added
fabric_route_aggregation_rules Data Sources and Resource added
Documentation and Acceptance Tests added for above resources and data sources

…Data Sources and Resources

codecov · 2025-02-18T22:54:01Z

Codecov Report

Attention: Patch coverage is 68.30189% with 336 lines in your changes missing coverage. Please review.

Project coverage is 65.37%. Comparing base (a5bf346) to head (e81fc08).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
...rces/fabric/connectionrouteaggregation/resource.go	7.24%	128 Missing ⚠️
.../resources/fabric/routeaggregationrule/resource.go	61.22%	56 Missing and 20 partials ⚠️
...ources/fabric/connectionrouteaggregation/models.go	0.00%	51 Missing ⚠️
...tion/datasource_by_connectionRouteAggregationId.go	30.30%	23 Missing ⚠️
...tion/datasource_all_connectionRouteAggregations.go	31.25%	22 Missing ⚠️
...gationrule/datasource_all_routeAggregationRules.go	64.00%	13 Missing and 5 partials ⚠️
...gationrule/datasource_by_routeAggregationRuleId.go	66.66%	8 Missing and 3 partials ⚠️
...al/resources/fabric/routeaggregationrule/models.go	91.46%	5 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #860       +/-   ##
===========================================
+ Coverage   32.63%   65.37%   +32.74%     
===========================================
  Files         205      217       +12     
  Lines       26607    27667     +1060     
===========================================
+ Hits         8682    18087     +9405     
+ Misses      17778     8694     -9084     
- Partials      147      886      +739

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

thogarty

Changes requested

thogarty · 2025-02-25T23:55:45Z

internal/resources/fabric/routeaggregationrule/resource_test.go

+			}
+
+			// Handle specific API error messages
+			var apiErr *fabricv4.GenericOpenAPIError


This error handling likely is redundant and not necessary. There also should be an easier way to get the errorCode back from the err.

See

terraform-provider-equinix/equinix/resource_fabric_cloud_router.go

Lines 564 to 575 in 4b61c51

_, err := client.CloudRoutersApi.DeleteCloudRouterByUuid(ctx, d.Id()).Execute()

if err != nil {

if genericError, ok := err.(*fabricv4.GenericOpenAPIError); ok {

if fabricErrs, ok := genericError.Model().([]fabricv4.Error); ok {

// EQ-3040055 = There is an existing update in REQUESTED state

if equinix_errors.HasErrorCode(fabricErrs, "EQ-3040055") {

return diags

}

}

}

return diag.FromErr(equinix_errors.FormatFabricError(err))

}

for an example of how to simplify this

Should still double check for the necessity of this though. It doesn't seem like it should be needed.

We need to fetch the errorCode and I have updated the method based on the above example

internal/resources/fabric/routeaggregationrule/resource.go

thogarty · 2025-02-26T00:04:18Z

internal/resources/fabric/routeaggregationrule/resource.go

+
+	newPrefix, oldPrefix := plan.Prefix.ValueString(), state.Prefix.ValueString()
+
+	if newPrefix == oldPrefix {


Our lack of plan modification here is going to cause this warning to be triggered every time this happens. Prepare for an update if issues arise.

thogarty · 2025-02-26T00:04:53Z

internal/resources/fabric/routeaggregationrule/resource.go

+	createWaiter := getCreateUpdateWaiter(ctx, client, routeAggregationID, routeAggregationRule.GetUuid(), createTimeout)
+	routeAggregationRuleChecked, err := createWaiter.WaitForStateContext(ctx)
+	if err != nil {
+		resp.Diagnostics.AddError(fmt.Sprintf("Failed creating Route Aggregation %s", routeAggregationRule.GetUuid()), err.Error())


nit: rule missing from error message

thogarty · 2025-02-26T00:09:00Z

internal/resources/fabric/connectionrouteaggregation/resource_test.go

+		routeAggregationId := rs.Primary.Attributes["route_aggregation_id"]
+		connectionId := rs.Primary.Attributes["connection_id"]
+
+		if connectionRouteAggregation, _, err := client.RouteAggregationsApi.GetConnectionRouteAggregationByUuid(ctx, routeAggregationId, connectionId).Execute(); err == nil &&


This pattern is better for a test check delete method.

thogarty · 2025-02-26T00:09:40Z

internal/resources/fabric/connectionrouteaggregation/resource.go

+	// deletedMarker is a terraform-provider-only value that is used by the waiter
+	// to indicate that the resource appears to be deleted successfully based on
+	// status code or specific error code
+	deletedMarker := "tf-marker-for-deleted-route-aggregation-rule"


nit: should be connection route aggregation string instead

thogarty · 2025-02-26T00:11:54Z

internal/resources/fabric/connectionrouteaggregation/resource_schema.go

+				Update: true,
+				Delete: true,
+			}),
+			"route_aggregation_id": schema.StringAttribute{


If this changes it should force a new resource. Same with connection_id

thogarty · 2025-02-26T00:12:08Z

internal/resources/fabric/connectionrouteaggregation/resource_schema.go

+				Description: "UUID of the Route Aggregation to apply this Rule to",
+				Required:    true,
+			},
+			"connection_id": schema.StringAttribute{


Should force a new resource if updated

thogarty

Just need to update the delete checks and then it's good to go 👍

thogarty · 2025-02-27T20:24:53Z

internal/resources/fabric/routeaggregationrule/resource.go

+			deletedMarker,
+		},
+		Refresh: func() (interface{}, string, error) {
+			routeAggregationRule, resp, err := client.RouteAggregationRulesApi.GetRouteAggregationRuleByUuid(ctx, routeAggregationID, id).Execute()


We don't need resp here.

thogarty · 2025-02-27T20:26:00Z

internal/resources/fabric/routeaggregationrule/resource.go

+		Refresh: func() (interface{}, string, error) {
+			routeAggregationRule, resp, err := client.RouteAggregationRulesApi.GetRouteAggregationRuleByUuid(ctx, routeAggregationID, id).Execute()
+			if err != nil {
+				if resp != nil {


Can remove resp check. Also, you should add the check for 404 status check back into this if statement and the one for resource_test.go. The simplification was about the conversion to GenericOpenAPIError and not about the status codes. Those were good and important checks.

thogarty

LGTM. Ship it.

github-actions · 2025-02-28T18:42:01Z

This PR is included in version 3.3.0 🎉

feat: Adding Route Aggregation Rule and Connection Route Aggregation …

b7ca341

…Data Sources and Resources

srushti-patl requested a review from a team as a code owner February 18, 2025 22:49

srushti-patl temporarily deployed to internal February 18, 2025 22:49 — with GitHub Actions Inactive

thogarty added the area/resources/fabric Issues related to Fabric and ECX APIs label Feb 20, 2025

fix: Resolving Merge Conflicts

a561571

srushti-patl temporarily deployed to internal February 25, 2025 19:28 — with GitHub Actions Inactive

srushti-patl temporarily deployed to internal February 25, 2025 23:19 — with GitHub Actions Inactive

fix: Resolving lint errors

5151372

srushti-patl force-pushed the CXF-106516-Route-Aggregation-Rule-Connection branch from 507dcc4 to 5151372 Compare February 25, 2025 23:25

srushti-patl deployed to internal February 25, 2025 23:25 — with GitHub Actions Active

srushti-patl temporarily deployed to internal February 25, 2025 23:25 — with GitHub Actions Inactive

thogarty requested changes Feb 26, 2025

View reviewed changes

fix: Addressing PR comments

3edca71

srushti-patl temporarily deployed to internal February 26, 2025 22:59 — with GitHub Actions Inactive

srushti-patl requested a review from thogarty February 26, 2025 23:00

thogarty requested changes Feb 27, 2025

View reviewed changes

fix: Updating code based on PR comments

8ff5eb1

srushti-patl temporarily deployed to internal February 27, 2025 23:21 — with GitHub Actions Inactive

thogarty approved these changes Feb 27, 2025

View reviewed changes

Merge branch 'main' into CXF-106516-Route-Aggregation-Rule-Connection

e81fc08

srushti-patl temporarily deployed to internal February 27, 2025 23:27 — with GitHub Actions Inactive

srushti-patl merged commit 6a231ed into main Feb 28, 2025
11 of 14 checks passed

srushti-patl deleted the CXF-106516-Route-Aggregation-Rule-Connection branch February 28, 2025 00:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adding Route Aggregation Rule and Connection Route Aggregation Data Sources and Resources #860

feat: Adding Route Aggregation Rule and Connection Route Aggregation Data Sources and Resources #860

srushti-patl commented Feb 18, 2025

codecov bot commented Feb 18, 2025 •

edited

Loading

thogarty left a comment

thogarty Feb 25, 2025

thogarty Feb 25, 2025

thogarty Feb 26, 2025

srushti-patl Feb 26, 2025

thogarty Feb 26, 2025

thogarty Feb 26, 2025

thogarty Feb 26, 2025

thogarty Feb 26, 2025

thogarty Feb 26, 2025

thogarty Feb 26, 2025

thogarty left a comment

thogarty Feb 27, 2025

thogarty Feb 27, 2025

thogarty left a comment

github-actions bot commented Feb 28, 2025

	_, err := client.CloudRoutersApi.DeleteCloudRouterByUuid(ctx, d.Id()).Execute()
	if err != nil {
	if genericError, ok := err.(*fabricv4.GenericOpenAPIError); ok {
	if fabricErrs, ok := genericError.Model().([]fabricv4.Error); ok {
	// EQ-3040055 = There is an existing update in REQUESTED state
	if equinix_errors.HasErrorCode(fabricErrs, "EQ-3040055") {
	return diags
	}
	}
	}
	return diag.FromErr(equinix_errors.FormatFabricError(err))
	}


		newPrefix, oldPrefix := plan.Prefix.ValueString(), state.Prefix.ValueString()

		if newPrefix == oldPrefix {

feat: Adding Route Aggregation Rule and Connection Route Aggregation Data Sources and Resources #860

feat: Adding Route Aggregation Rule and Connection Route Aggregation Data Sources and Resources #860

Conversation

srushti-patl commented Feb 18, 2025

codecov bot commented Feb 18, 2025 • edited Loading

Codecov Report

thogarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thogarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thogarty left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 28, 2025

codecov bot commented Feb 18, 2025 •

edited

Loading