fix: correct consumers validation when custom_id is used (#1012)

Kong · Sep 6, 2023 · 55fb115 · 55fb115
1 parent 02db446
commit 55fb115
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 16 deletions.
diff --git a/file/builder.go b/file/builder.go
@@ -289,8 +289,14 @@ func (b *stateBuilder) consumers() {
 	for _, c := range b.targetContent.Consumers {
 		c := c
 		if utils.Empty(c.ID) {
-			consumer, err := b.currentState.Consumers.GetByIDOrUsername(*c.Username)
-			if errors.Is(err, state.ErrNotFound) {
+			var (
+				consumer *state.Consumer
+				err      error
+			)
+			if c.Username != nil {
+				consumer, err = b.currentState.Consumers.GetByIDOrUsername(*c.Username)
+			}
+			if errors.Is(err, state.ErrNotFound) || consumer == nil {
 				if c.CustomID != nil {
 					consumer, err = b.currentState.Consumers.GetByCustomID(*c.CustomID)
 					if err == nil {

diff --git a/file/codegen/main.go b/file/codegen/main.go
@@ -22,12 +22,15 @@ var (
 		},
 	}
 
-	anyOfUsernameOrID = []*jsonschema.Type{
+	// consumers
+	anyOfUsernameOrCustomID = []*jsonschema.Type{
 		{
-			Required: []string{"username"},
+			Description: "at least one of custom_id or username must be set",
+			Required:    []string{"username"},
 		},
 		{
-			Required: []string{"id"},
+			Description: "at least one of custom_id or username must be set",
+			Required:    []string{"custom_id"},
 		},
 	}
 )
@@ -52,7 +55,7 @@ func main() {
 
 	schema.Definitions["FRoute"].AnyOf = anyOfNameOrID
 
-	schema.Definitions["FConsumer"].AnyOf = anyOfUsernameOrID
+	schema.Definitions["FConsumer"].AnyOf = anyOfUsernameOrCustomID
 
 	schema.Definitions["FUpstream"].Required = []string{"name"}
 

diff --git a/file/kong_json_schema.json b/file/kong_json_schema.json
diff --git a/tests/integration/sync_test.go b/tests/integration/sync_test.go
@@ -4137,7 +4137,7 @@ func Test_Sync_CreateCertificateWithSNIs(t *testing.T) {
 // test scope:
 //   - 3.0.0+
 //   - konnect
-func Test_Sync_ConsumersWithCustomIDAndUsername(t *testing.T) {
+func Test_Sync_ConsumersWithCustomIDAndOrUsername(t *testing.T) {
 	runWhenKongOrKonnect(t, ">=3.0.0")
 	setup(t)
 
@@ -4161,6 +4161,14 @@ func Test_Sync_ConsumersWithCustomIDAndUsername(t *testing.T) {
 				Username: kong.String("foo"),
 				CustomID: kong.String("bar"),
 			},
+			{
+				ID:       kong.String("18c62c3c-12cc-429a-8e5a-57f2c3691a6b"),
+				CustomID: kong.String("custom_id_only"),
+			},
+			{
+				ID:       kong.String("8ef278c9-48c1-43e1-b665-e9bc18fab4c8"),
+				Username: kong.String("username_only"),
+			},
 		},
 	}, nil)
 }

diff --git a/tests/integration/test_utils.go b/tests/integration/test_utils.go
@@ -95,8 +95,16 @@ func sortSlices(x, y interface{}) bool {
 		yName = *yEntity.Prefix
 	case *kong.Consumer:
 		yEntity := y.(*kong.Consumer)
-		xName = *xEntity.Username
-		yName = *yEntity.Username
+		if xEntity.Username != nil {
+			xName = *xEntity.Username
+		} else {
+			xName = *xEntity.ID
+		}
+		if yEntity.Username != nil {
+			yName = *yEntity.Username
+		} else {
+			yName = *yEntity.ID
+		}
 	case *kong.ConsumerGroup:
 		yEntity := y.(*kong.ConsumerGroup)
 		xName = *xEntity.Name

diff --git a/tests/integration/testdata/sync/024-consumers-with-custom_id-and-username/kong3x.yaml b/tests/integration/testdata/sync/024-consumers-with-custom_id-and-username/kong3x.yaml
@@ -6,3 +6,7 @@ consumers:
 - custom_id: bar
   id: 7820f383-7b77-4fcc-af7f-14ff3e256693
   username: foo
+- custom_id: custom_id_only
+  id: 18c62c3c-12cc-429a-8e5a-57f2c3691a6b
+- id: 8ef278c9-48c1-43e1-b665-e9bc18fab4c8
+  username: username_only
diff --git a/types/consumer.go b/types/consumer.go
@@ -181,12 +181,38 @@ func (d *consumerDiffer) DuplicatesDeletes() ([]crud.Event, error) {
 }
 
 func (d *consumerDiffer) deleteDuplicateConsumer(targetConsumer *state.Consumer) (*crud.Event, error) {
-	currentConsumer, err := d.currentState.Consumers.GetByIDOrUsername(*targetConsumer.Username)
-	if errors.Is(err, state.ErrNotFound) {
-		return nil, nil
+	var (
+		idOrUsername string
+
+		currentConsumer *state.Consumer
+		err             error
+	)
+
+	if targetConsumer.Username != nil {
+		idOrUsername = *targetConsumer.Username
+	} else if targetConsumer.ID != nil {
+		idOrUsername = *targetConsumer.ID
+	}
+
+	if idOrUsername != "" {
+		currentConsumer, err = d.currentState.Consumers.GetByIDOrUsername(idOrUsername)
+	}
+	if errors.Is(err, state.ErrNotFound) || idOrUsername == "" {
+		if targetConsumer.CustomID != nil {
+			currentConsumer, err = d.currentState.Consumers.GetByCustomID(*targetConsumer.CustomID)
+			if errors.Is(err, state.ErrNotFound) {
+				return nil, nil
+			}
+			if err != nil {
+				return nil, fmt.Errorf("error looking up consumer by custom_id %q: %w",
+					*targetConsumer.Username, err)
+			}
+		} else {
+			return nil, nil
+		}
 	}
 	if err != nil {
-		return nil, fmt.Errorf("error looking up consumer %q: %w",
+		return nil, fmt.Errorf("error looking up consumer by username or id %q: %w",
 			*targetConsumer.Username, err)
 	}