Skip to content

Commit adf3daf

Browse files
committed
Handle zero weights in cluster and non-cluster mappings, add new tests
- previously zero weights were interpreted as being 100 - added tests verifying the partial weights with 0 entries - deprecated `GetWeight` method in favor of directly using the `Weight` field (this is the function that made 0 -> 100) Signed-off-by: Alberto Ricart <[email protected]>
1 parent a2c583b commit adf3daf

File tree

2 files changed

+43
-5
lines changed

2 files changed

+43
-5
lines changed

v2/account_claims.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,9 @@ type WeightedMapping struct {
140140
Cluster string `json:"cluster,omitempty"`
141141
}
142142

143+
// GetWeight returns the weight value.
144+
// Deprecated: use Weight field directly.
143145
func (m *WeightedMapping) GetWeight() uint8 {
144-
if m.Weight == 0 {
145-
return 100
146-
}
147146
return m.Weight
148147
}
149148

@@ -164,7 +163,7 @@ func (m *Mapping) Validate(vr *ValidationResults) {
164163
vr.AddError("Mapping %q in cluster %q exceeds 100%% among all of it's weighted to mappings", ubFrom, e.Cluster)
165164
}
166165
} else {
167-
total += e.GetWeight()
166+
total += e.Weight
168167
}
169168
}
170169
if total > 100 {

v2/account_claims_test.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ func TestAccountMapping(t *testing.T) { // don't block encoding!!!
685685
vr = &ValidationResults{}
686686
account.Mappings = Mapping{}
687687
account.AddMapping("foo4",
688-
WeightedMapping{Subject: "to1"}, // no weight means 100
688+
WeightedMapping{Subject: "to1", Weight: 100},
689689
WeightedMapping{Subject: "to2", Weight: 1})
690690
account.Validate(vr)
691691
if !vr.IsBlocking(false) {
@@ -743,6 +743,45 @@ func TestAccountClusterNoOver100Mapping(t *testing.T) { // don't block encoding!
743743
}
744744
}
745745

746+
func TestAccountClusterMappingWithZeroWeights(t *testing.T) {
747+
akp := createAccountNKey(t)
748+
apk := publicKey(akp, t)
749+
750+
account := NewAccountClaims(apk)
751+
vr := &ValidationResults{}
752+
753+
// multiple mappings with weight 0 should be allowed and not count toward total
754+
account.AddMapping("q",
755+
WeightedMapping{Subject: "qq", Weight: 0, Cluster: "A"}, // 0 weight in cluster A
756+
WeightedMapping{Subject: "bb", Weight: 100, Cluster: "A"}, // 100 weight in cluster A (total: 100)
757+
WeightedMapping{Subject: "cc", Weight: 0, Cluster: "B"}, // 0 weight in cluster B
758+
WeightedMapping{Subject: "dd", Weight: 0, Cluster: "B"}, // another 0 weight in cluster B
759+
WeightedMapping{Subject: "ee", Weight: 50, Cluster: "B"}, // 50 weight in cluster B (total: 50)
760+
WeightedMapping{Subject: "ff", Weight: 0}, // 0 weight non-cluster
761+
WeightedMapping{Subject: "gg", Weight: 50}) // 50 weight non-cluster (total: 50)
762+
account.Validate(vr)
763+
if !vr.IsEmpty() {
764+
t.Fatal("Expected no errors")
765+
}
766+
}
767+
768+
func TestAccountMappingWith30And0Weights(t *testing.T) {
769+
akp := createAccountNKey(t)
770+
apk := publicKey(akp, t)
771+
772+
account := NewAccountClaims(apk)
773+
vr := &ValidationResults{}
774+
775+
// weight 30 + weight 0 = 30, should be valid
776+
account.AddMapping("q",
777+
WeightedMapping{Subject: "qq", Weight: 30},
778+
WeightedMapping{Subject: "bb", Weight: 0})
779+
account.Validate(vr)
780+
if !vr.IsEmpty() {
781+
t.Fatal("Expected no errors")
782+
}
783+
}
784+
746785
func TestAccountExternalAuthorization(t *testing.T) {
747786
akp := createAccountNKey(t)
748787
apk := publicKey(akp, t)

0 commit comments

Comments
 (0)