Skip to content

Commit 2da8164

Browse files
gilmor-portgilmor-port
authored
Merge pull request #299 from port-labs/PORT-16658-sort-rules-fix
Port 16658 - SCAB - sort rules fix
2 parents e08e593 + 185aff3 commit 2da8164

File tree

3 files changed

+233
-16
lines changed

3 files changed

+233
-16
lines changed

‎port/scorecard/refreshScorecardState.go‎

Lines changed: 81 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66
"reflect"
7-
"sort"
87

98
"github.com/hashicorp/terraform-plugin-framework/types"
109
"github.com/port-labs/terraform-provider-port-labs/v2/internal/cli"
@@ -116,13 +115,88 @@ func (r *ScorecardResource) refreshScorecardState(ctx context.Context, state *Sc
116115
stateRules = append(stateRules, *stateRule)
117116
}
118117

119-
// Sort rules by identifier to prevent Terraform from detecting false-positive changes
120-
// due to API returning rules in different order
121-
sort.Slice(stateRules, func(i, j int) bool {
122-
return stateRules[i].Identifier.ValueString() < stateRules[j].Identifier.ValueString()
123-
})
118+
// Preserve the original order from state if it exists
119+
if len(state.Rules) > 0 {
120+
// Create a map of API rule identifier to API rule for quick lookup
121+
apiRulesByIdentifier := make(map[string]*cli.Rule)
122+
for i := range s.Rules {
123+
apiRulesByIdentifier[s.Rules[i].Identifier] = &s.Rules[i]
124+
}
125+
126+
// Process existing rules in their original order, updating with API data
127+
orderedRules := make([]Rule, 0, len(stateRules))
128+
processedIdentifiers := make(map[string]bool)
129+
130+
// First, update existing rules in their original order
131+
for _, existingRule := range state.Rules {
132+
identifier := existingRule.Identifier.ValueString()
133+
if apiRule, exists := apiRulesByIdentifier[identifier]; exists {
134+
// Update the existing rule with fresh API data while preserving structure
135+
updatedRule := Rule{
136+
Title: types.StringValue(apiRule.Title),
137+
Level: types.StringValue(apiRule.Level),
138+
Identifier: types.StringValue(apiRule.Identifier),
139+
}
140+
141+
// Handle description - preserve from state if it exists, otherwise use API value
142+
if !existingRule.Description.IsNull() && !existingRule.Description.IsUnknown() {
143+
updatedRule.Description = existingRule.Description
144+
} else if apiRule.Description != "" {
145+
updatedRule.Description = types.StringValue(apiRule.Description)
146+
} else {
147+
updatedRule.Description = types.StringNull()
148+
}
149+
150+
// Update query from API
151+
stateQuery := &Query{
152+
Combinator: types.StringValue(apiRule.Query.Combinator),
153+
}
154+
stateQuery.Conditions = make([]types.String, len(apiRule.Query.Conditions))
155+
for i, u := range apiRule.Query.Conditions {
156+
cond, _ := utils.GoObjectToTerraformString(u, r.portClient.JSONEscapeHTML)
157+
stateQuery.Conditions[i] = cond
158+
}
159+
updatedRule.Query = stateQuery
160+
161+
orderedRules = append(orderedRules, updatedRule)
162+
processedIdentifiers[identifier] = true
163+
}
164+
}
124165

125-
state.Rules = stateRules
166+
// Then append any new rules from API that weren't in the original state
167+
for _, apiRule := range s.Rules {
168+
if !processedIdentifiers[apiRule.Identifier] {
169+
newRule := Rule{
170+
Title: types.StringValue(apiRule.Title),
171+
Level: types.StringValue(apiRule.Level),
172+
Identifier: types.StringValue(apiRule.Identifier),
173+
}
174+
175+
if apiRule.Description != "" {
176+
newRule.Description = types.StringValue(apiRule.Description)
177+
} else {
178+
newRule.Description = types.StringNull()
179+
}
180+
181+
stateQuery := &Query{
182+
Combinator: types.StringValue(apiRule.Query.Combinator),
183+
}
184+
stateQuery.Conditions = make([]types.String, len(apiRule.Query.Conditions))
185+
for i, u := range apiRule.Query.Conditions {
186+
cond, _ := utils.GoObjectToTerraformString(u, r.portClient.JSONEscapeHTML)
187+
stateQuery.Conditions[i] = cond
188+
}
189+
newRule.Query = stateQuery
190+
191+
orderedRules = append(orderedRules, newRule)
192+
}
193+
}
194+
195+
state.Rules = orderedRules
196+
} else {
197+
// No existing state, use API order
198+
state.Rules = stateRules
199+
}
126200
if shouldRefreshLevels(state.Levels, s.Levels) {
127201
state.Levels = fromCliLevelsToTerraformLevels(s.Levels)
128202
}

‎port/scorecard/resource_test.go‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,3 +522,151 @@ func TestAccPortScorecardUpdateIdentifier(t *testing.T) {
522522
},
523523
})
524524
}
525+
526+
func TestAccPortScorecardRuleOrderPreservation(t *testing.T) {
527+
blueprintIdentifier := utils.GenID()
528+
scorecardIdentifier := utils.GenID()
529+
530+
// Create scorecard with rules in a specific non-alphabetical order
531+
// Order: "zebra" (Z), "alpha" (A), "beta" (B)
532+
// This tests that order is preserved even if API returns them alphabetically
533+
var testAccConfigCreate = testAccCreateBlueprintConfig(blueprintIdentifier) + fmt.Sprintf(`
534+
resource "port_scorecard" "test" {
535+
identifier = "%s"
536+
title = "Rule Order Test"
537+
blueprint = "%s"
538+
rules = [
539+
{
540+
identifier = "zebra"
541+
title = "Zebra Rule"
542+
level = "Gold"
543+
query = {
544+
combinator = "and"
545+
conditions = [jsonencode({
546+
property = "$team"
547+
operator = "isNotEmpty"
548+
})]
549+
}
550+
},
551+
{
552+
identifier = "alpha"
553+
title = "Alpha Rule"
554+
level = "Silver"
555+
query = {
556+
combinator = "and"
557+
conditions = [jsonencode({
558+
property = "author"
559+
operator = "isNotEmpty"
560+
})]
561+
}
562+
},
563+
{
564+
identifier = "beta"
565+
title = "Beta Rule"
566+
level = "Bronze"
567+
query = {
568+
combinator = "and"
569+
conditions = [jsonencode({
570+
property = "url"
571+
operator = "isNotEmpty"
572+
})]
573+
}
574+
}
575+
]
576+
depends_on = [
577+
port_blueprint.microservice
578+
]
579+
}`, scorecardIdentifier, blueprintIdentifier)
580+
581+
// Update config - only change title to trigger refresh, rules remain the same
582+
var testAccConfigUpdate = testAccCreateBlueprintConfig(blueprintIdentifier) + fmt.Sprintf(`
583+
resource "port_scorecard" "test" {
584+
identifier = "%s"
585+
title = "Rule Order Test Updated"
586+
blueprint = "%s"
587+
rules = [
588+
{
589+
identifier = "zebra"
590+
title = "Zebra Rule"
591+
level = "Gold"
592+
query = {
593+
combinator = "and"
594+
conditions = [jsonencode({
595+
property = "$team"
596+
operator = "isNotEmpty"
597+
})]
598+
}
599+
},
600+
{
601+
identifier = "alpha"
602+
title = "Alpha Rule"
603+
level = "Silver"
604+
query = {
605+
combinator = "and"
606+
conditions = [jsonencode({
607+
property = "author"
608+
operator = "isNotEmpty"
609+
})]
610+
}
611+
},
612+
{
613+
identifier = "beta"
614+
title = "Beta Rule"
615+
level = "Bronze"
616+
query = {
617+
combinator = "and"
618+
conditions = [jsonencode({
619+
property = "url"
620+
operator = "isNotEmpty"
621+
})]
622+
}
623+
}
624+
]
625+
depends_on = [
626+
port_blueprint.microservice
627+
]
628+
}`, scorecardIdentifier, blueprintIdentifier)
629+
630+
resource.Test(t, resource.TestCase{
631+
PreCheck: func() { acctest.TestAccPreCheck(t) },
632+
ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
633+
Steps: []resource.TestStep{
634+
{
635+
Config: acctest.ProviderConfig + testAccConfigCreate,
636+
Check: resource.ComposeTestCheckFunc(
637+
resource.TestCheckResourceAttr("port_scorecard.test", "title", "Rule Order Test"),
638+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.#", "3"),
639+
// Verify rules are in the original order (zebra, alpha, beta)
640+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.0.identifier", "zebra"),
641+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.0.title", "Zebra Rule"),
642+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.0.level", "Gold"),
643+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.1.identifier", "alpha"),
644+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.1.title", "Alpha Rule"),
645+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.1.level", "Silver"),
646+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.2.identifier", "beta"),
647+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.2.title", "Beta Rule"),
648+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.2.level", "Bronze"),
649+
),
650+
},
651+
{
652+
Config: acctest.ProviderConfig + testAccConfigUpdate,
653+
Check: resource.ComposeTestCheckFunc(
654+
resource.TestCheckResourceAttr("port_scorecard.test", "title", "Rule Order Test Updated"),
655+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.#", "3"),
656+
// Verify rules are STILL in the original order after refresh (zebra, alpha, beta)
657+
// This ensures that even if API returns them in a different order,
658+
// the original state order is preserved
659+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.0.identifier", "zebra"),
660+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.0.title", "Zebra Rule"),
661+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.0.level", "Gold"),
662+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.1.identifier", "alpha"),
663+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.1.title", "Alpha Rule"),
664+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.1.level", "Silver"),
665+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.2.identifier", "beta"),
666+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.2.title", "Beta Rule"),
667+
resource.TestCheckResourceAttr("port_scorecard.test", "rules.2.level", "Bronze"),
668+
),
669+
},
670+
},
671+
})
672+
}

‎port/scorecard/scorecardResourceToPortBody.go‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package scorecard
33
import (
44
"context"
55
"encoding/json"
6-
"sort"
76

87
"github.com/port-labs/terraform-provider-port-labs/v2/internal/cli"
98
)
@@ -46,17 +45,13 @@ func scorecardResourceToPortBody(ctx context.Context, state *ScorecardModel) (*c
4645
s.Filter = filter
4746
}
4847

49-
// Sort rules by identifier to ensure consistent ordering
50-
sortedStateRules := make([]Rule, len(state.Rules))
51-
copy(sortedStateRules, state.Rules)
52-
53-
sort.Slice(sortedStateRules, func(i, j int) bool {
54-
return sortedStateRules[i].Identifier.ValueString() < sortedStateRules[j].Identifier.ValueString()
55-
})
48+
// Use rules in the order defined in Terraform state
49+
stateRules := make([]Rule, len(state.Rules))
50+
copy(stateRules, state.Rules)
5651

5752
var rules []cli.Rule
5853

59-
for _, stateRule := range sortedStateRules {
54+
for _, stateRule := range stateRules {
6055
rule := &cli.Rule{
6156
Level: stateRule.Level.ValueString(),
6257
Identifier: stateRule.Identifier.ValueString(),

0 commit comments

Comments
 (0)