From 0ad4826fabf1bdc906d30b092b6d0e3ebf68729f Mon Sep 17 00:00:00 2001 From: Oscar Zhou <100548325+oscarzhou-portainer@users.noreply.github.com> Date: Fri, 8 Mar 2024 09:56:10 +1300 Subject: [PATCH] fix(stack): filter out orphan stacks that have same name as normal stacks [EE-6791] (#11291) --- api/http/handler/stacks/stack_list.go | 44 +++++++++++-- api/http/handler/stacks/stack_list_test.go | 74 ++++++++++++++++++++++ 2 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 api/http/handler/stacks/stack_list_test.go diff --git a/api/http/handler/stacks/stack_list.go b/api/http/handler/stacks/stack_list.go index feba50843..32caf5281 100644 --- a/api/http/handler/stacks/stack_list.go +++ b/api/http/handler/stacks/stack_list.go @@ -91,25 +91,55 @@ func (handler *Handler) stackList(w http.ResponseWriter, r *http.Request) *httpe return response.JSON(w, stacks) } +// filterStacks refines a collection of Stack instances using specified criteria. +// This function examines the provided filters: EndpointID, SwarmID, and IncludeOrphanedStacks. +// - If both EndpointID is zero and SwarmID is an empty string, the function directly returns the original stack list without any modifications. +// - If either filter is specified, it proceeds to selectively include stacks that match the criteria. + +// Key Points on Business Logic: +// 1. Determining Inclusion of Orphaned Stacks: +// - The decision to include orphaned stacks is influenced by the user's role and usually set by the client (UI). +// - Administrators or environment administrators can include orphaned stacks by setting IncludeOrphanedStacks to true, reflecting their broader access rights. +// - For non-administrative users, this is typically set to false, limiting their visibility to only stacks within their purview. + +// 2. Inclusion Criteria for Orphaned Stacks: +// - When IncludeOrphanedStacks is true and an EndpointID is specified (not zero), the function selects: +// a) Stacks linked to the specified EndpointID. +// b) Orphaned stacks that don't have a naming conflict with any stack associated with the EndpointID. +// - This approach is designed to avoid name conflicts within Docker Compose, which restricts the creation of multiple stacks with the same name. + +// 3. Type Matching for Orphaned Stacks: +// - The function ensures that orphaned stacks are compatible with the environment's stack type (compose or swarm). +// - It filters out orphaned swarm stacks in Docker standalone environments +// - It filters out orphaned standalone stack in Docker swarm environments +// - This ensures that re-association respects the constraints of the environment and stack type. + +// The outcome is a new list of stacks that align with these filtering and business logic criteria. func filterStacks(stacks []portainer.Stack, filters *stackListOperationFilters, endpoints []portainer.Endpoint) []portainer.Stack { if filters.EndpointID == 0 && filters.SwarmID == "" { return stacks } filteredStacks := make([]portainer.Stack, 0, len(stacks)) + uniqueStackNames := make(map[string]struct{}) for _, stack := range stacks { - if filters.IncludeOrphanedStacks && isOrphanedStack(stack, endpoints) { - if (stack.Type == portainer.DockerComposeStack && filters.SwarmID == "") || (stack.Type == portainer.DockerSwarmStack && filters.SwarmID != "") { - filteredStacks = append(filteredStacks, stack) - } - continue - } - if stack.Type == portainer.DockerComposeStack && stack.EndpointID == portainer.EndpointID(filters.EndpointID) { filteredStacks = append(filteredStacks, stack) + uniqueStackNames[stack.Name] = struct{}{} } if stack.Type == portainer.DockerSwarmStack && stack.SwarmID == filters.SwarmID { filteredStacks = append(filteredStacks, stack) + uniqueStackNames[stack.Name] = struct{}{} + } + } + + for _, stack := range stacks { + if filters.IncludeOrphanedStacks && isOrphanedStack(stack, endpoints) { + if (stack.Type == portainer.DockerComposeStack && filters.SwarmID == "") || (stack.Type == portainer.DockerSwarmStack && filters.SwarmID != "") { + if _, exists := uniqueStackNames[stack.Name]; !exists { + filteredStacks = append(filteredStacks, stack) + } + } } } diff --git a/api/http/handler/stacks/stack_list_test.go b/api/http/handler/stacks/stack_list_test.go new file mode 100644 index 000000000..bad5dfae8 --- /dev/null +++ b/api/http/handler/stacks/stack_list_test.go @@ -0,0 +1,74 @@ +package stacks + +import ( + "sort" + "testing" + + portainer "github.com/portainer/portainer/api" + "github.com/stretchr/testify/assert" +) + +func TestFilterStacks(t *testing.T) { + t.Run("filter stacks against particular endpoint and all orphaned stacks", func(t *testing.T) { + stacks := []portainer.Stack{ + {ID: 1, EndpointID: 3, Name: "normal_stack", Type: portainer.DockerComposeStack}, + {ID: 2, EndpointID: 4, Name: "orphaned_stack", Type: portainer.DockerComposeStack}, + {ID: 3, EndpointID: 5, Name: "other_stack", Type: portainer.DockerComposeStack}, + } + filters := &stackListOperationFilters{EndpointID: 3, IncludeOrphanedStacks: true} + endpoints := []portainer.Endpoint{{ID: 3}, {ID: 5}} + + expectStacks := []portainer.Stack{{ID: 1}, {ID: 2}} + actualStacks := filterStacks(stacks, filters, endpoints) + + isEqualStacks(t, expectStacks, actualStacks) + }) + + t.Run("filter unique stacks against particular endpoint and all orphaned stacks and an orphaned stack has the same name with normal stack", func(t *testing.T) { + stacks := []portainer.Stack{ + {ID: 1, EndpointID: 3, Name: "normal_stack", Type: portainer.DockerComposeStack}, + {ID: 2, EndpointID: 4, Name: "orphaned_stack", Type: portainer.DockerComposeStack}, + {ID: 3, EndpointID: 5, Name: "other_stack", Type: portainer.DockerComposeStack}, + {ID: 4, EndpointID: 4, Name: "normal_stack", Type: portainer.DockerComposeStack}, + } + filters := &stackListOperationFilters{EndpointID: 3, IncludeOrphanedStacks: true} + endpoints := []portainer.Endpoint{{ID: 3}, {ID: 5}} + + expectStacks := []portainer.Stack{{ID: 1}, {ID: 2}} + actualStacks := filterStacks(stacks, filters, endpoints) + + isEqualStacks(t, expectStacks, actualStacks) + }) + + t.Run("only filter stacks against particular endpoint and no orphaned stacks", func(t *testing.T) { + stacks := []portainer.Stack{ + {ID: 1, EndpointID: 3, Name: "normal_stack", Type: portainer.DockerComposeStack}, + {ID: 2, EndpointID: 4, Name: "orphaned_stack", Type: portainer.DockerComposeStack}, + {ID: 3, EndpointID: 5, Name: "other_stack", Type: portainer.DockerComposeStack}, + {ID: 4, EndpointID: 4, Name: "normal_stack", Type: portainer.DockerComposeStack}, + } + filters := &stackListOperationFilters{EndpointID: 3, IncludeOrphanedStacks: false} + endpoints := []portainer.Endpoint{{ID: 3}, {ID: 5}} + + expectStacks := []portainer.Stack{{ID: 1}} + actualStacks := filterStacks(stacks, filters, endpoints) + + isEqualStacks(t, expectStacks, actualStacks) + }) +} + +func isEqualStacks(t *testing.T, expectStacks, actualStacks []portainer.Stack) { + expectStackIDs := make([]int, len(expectStacks)) + for i, stack := range expectStacks { + expectStackIDs[i] = int(stack.ID) + } + sort.Ints(expectStackIDs) + + actualStackIDs := make([]int, len(actualStacks)) + for i, stack := range actualStacks { + actualStackIDs[i] = int(stack.ID) + } + sort.Ints(actualStackIDs) + + assert.Equal(t, expectStackIDs, actualStackIDs) +}