diff --git a/docs/release-control/v6/internal/subsystems/cloud-paid.md b/docs/release-control/v6/internal/subsystems/cloud-paid.md index 86f0329229..1a582b52b0 100644 --- a/docs/release-control/v6/internal/subsystems/cloud-paid.md +++ b/docs/release-control/v6/internal/subsystems/cloud-paid.md @@ -3521,6 +3521,12 @@ network, avoiding a same-host provider stack with the same role label. Regressio coverage is in `internal/cloudcp/health_monitor_test.go` and `internal/cloudcp/docker/manager_test.go`. +The same boundary applies when removing a client: cleanup only selects that +managed client's derived, correctly labelled tenant network, and only +disconnects support containers on this provider ingress that are still +attached to it. A role label by itself must not select another provider's +support container for a forced disconnect. + ### Provider MSP status reads client health that any caller can observe `provider-msp status`, which `upgrade.sh` gates on before every provider diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 4f12516420..6bc7bb2a9a 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -2462,6 +2462,15 @@ require exact tenant ownership and tenant-runtime labels on an existing isolated network before attaching containers; provisioning refuses an unlabelled same-named network instead of adopting it. A legacy client without an isolated network remains a reconnection no-op. +Client removal follows the same installation boundary: the Docker manager +identifies the managed client's network by its derived name and exact tenant +and runtime labels, not by a generic tenant-network label alone. It must force +disconnect only this provider's role-labelled support containers that are +still attached to that network, selecting them through the configured provider +ingress network. A recreated or other provider's support container is not a +cleanup target. `internal/cloudcp/docker/manager_test.go` covers the network +selection and disconnect filters; source-only coverage does not replace a +two-provider installed upgrade, recreation and cleanup check. ### Provider MSP operations accept a renewed licence diff --git a/internal/cloudcp/docker/manager.go b/internal/cloudcp/docker/manager.go index 8c1bbf669b..31a5a839be 100644 --- a/internal/cloudcp/docker/manager.go +++ b/internal/cloudcp/docker/manager.go @@ -453,10 +453,8 @@ func (m *Manager) ensureContainerConnectedToNetwork(ctx context.Context, network if err != nil { return fmt.Errorf("inspect tenant network %q: %w", networkName, err) } - for id := range inspect.Network.Containers { - if id == containerID || strings.HasPrefix(id, containerID) || strings.HasPrefix(containerID, id) { - return nil - } + if networkHasContainer(inspect.Network.Containers, containerID) { + return nil } _, err = m.cli.NetworkConnect(ctx, networkName, client.NetworkConnectOptions{ Container: containerID, @@ -465,6 +463,18 @@ func (m *Manager) ensureContainerConnectedToNetwork(ctx context.Context, network return err } +func networkHasContainer(containers map[string]network.EndpointResource, containerID string) bool { + if containerID == "" { + return false + } + for id := range containers { + if id != "" && (id == containerID || strings.HasPrefix(id, containerID) || strings.HasPrefix(containerID, id)) { + return true + } + } + return false +} + // IsNotFound reports whether Docker treated an identifier as missing. func IsNotFound(err error) bool { return errdefs.IsNotFound(err) @@ -949,12 +959,17 @@ func (m *Manager) tenantNetworkNamesForContainer(ctx context.Context, containerI return nil } inspect := inspectResult.Container - if inspect.NetworkSettings == nil { + if inspect.Config == nil || inspect.NetworkSettings == nil { return nil } + tenantID := strings.TrimSpace(inspect.Config.Labels["pulse.tenant.id"]) + if tenantID == "" || inspect.Config.Labels["pulse.managed"] != "true" { + return nil + } + ownedName := m.tenantNetworkName(tenantID) var out []string for networkName := range inspect.NetworkSettings.Networks { - if m.isTenantNetwork(ctx, networkName) { + if networkName == ownedName && m.isTenantNetwork(ctx, networkName, tenantID) { out = append(out, networkName) } } @@ -962,16 +977,16 @@ func (m *Manager) tenantNetworkNamesForContainer(ctx context.Context, containerI return out } -func (m *Manager) isTenantNetwork(ctx context.Context, networkName string) bool { +func (m *Manager) isTenantNetwork(ctx context.Context, networkName, tenantID string) bool { networkName = strings.TrimSpace(networkName) - if m == nil || networkName == "" { + if m == nil || networkName == "" || tenantID == "" { return false } inspect, err := m.cli.NetworkInspect(ctx, networkName, client.NetworkInspectOptions{}) if err != nil { return false } - return strings.TrimSpace(inspect.Network.Labels[tenantRuntimeNetworkLabel]) == tenantRuntimeNetworkLabelValue + return isOwnedTenantNetwork(inspect.Network.Labels, tenantID) } func (m *Manager) removeTenantNetwork(ctx context.Context, networkName string) error { @@ -991,6 +1006,14 @@ func (m *Manager) disconnectSupportContainersFromTenantNetwork(ctx context.Conte if m == nil || !m.cfg.IsolateTenantNetworks { return nil } + providerNetwork := strings.TrimSpace(m.cfg.Network) + if providerNetwork == "" { + return fmt.Errorf("provider ingress network is required to select support containers") + } + inspected, err := m.cli.NetworkInspect(ctx, networkName, client.NetworkInspectOptions{}) + if err != nil { + return fmt.Errorf("inspect tenant network %q for disconnect: %w", networkName, err) + } for _, label := range m.cfg.SupportContainerLabels { label = strings.TrimSpace(label) if label == "" { @@ -998,6 +1021,8 @@ func (m *Manager) disconnectSupportContainersFromTenantNetwork(ctx context.Conte } filters := client.Filters{} filters = filters.Add("label", label) + // The role label is shared by every provider installation on this host. + filters = filters.Add("network", providerNetwork) result, err := m.cli.ContainerList(ctx, client.ContainerListOptions{ All: true, Filters: filters, @@ -1006,7 +1031,9 @@ func (m *Manager) disconnectSupportContainersFromTenantNetwork(ctx context.Conte return fmt.Errorf("list provider support containers for disconnect label %q: %w", label, err) } for _, item := range result.Items { - if item.ID == "" { + // A replacement support container has not joined this tenant network. + // Do not try to disconnect it (or another provider's container). + if !networkHasContainer(inspected.Network.Containers, item.ID) { continue } _, err := m.cli.NetworkDisconnect(ctx, networkName, client.NetworkDisconnectOptions{ diff --git a/internal/cloudcp/docker/manager_test.go b/internal/cloudcp/docker/manager_test.go index ad238d2cb0..5dc320e137 100644 --- a/internal/cloudcp/docker/manager_test.go +++ b/internal/cloudcp/docker/manager_test.go @@ -737,3 +737,117 @@ func TestEnsureTenantNetworkRejectsUnownedExistingNetwork(t *testing.T) { t.Fatalf("ensureTenantNetwork(empty) = (%q, %v), want rejection", got, err) } } + +func TestTenantNetworkCleanupSelectsOnlyOwnedNetwork(t *testing.T) { + ownedLabels := map[string]string{"pulse.tenant.id": "t-acme", tenantRuntimeNetworkLabel: tenantRuntimeNetworkLabelValue} + var containerLabels, targetLabels map[string]string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Api-Version", "1.47") + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasSuffix(r.URL.Path, "/containers/client-a/json"): + _ = json.NewEncoder(w).Encode(map[string]any{ + "Id": "client-a", "Config": map[string]any{"Labels": containerLabels}, + "NetworkSettings": map[string]any{"Networks": map[string]any{ + "provider-a-tenant-t-acme": map[string]any{}, + "provider-b-tenant-t-acme": map[string]any{}, + }}, + }) + case strings.HasSuffix(r.URL.Path, "/networks/provider-a-tenant-t-acme"): + _ = json.NewEncoder(w).Encode(map[string]any{"Name": "provider-a-tenant-t-acme", "Labels": targetLabels}) + case strings.HasSuffix(r.URL.Path, "/networks/provider-b-tenant-t-acme"): + _ = json.NewEncoder(w).Encode(map[string]any{"Name": "provider-b-tenant-t-acme", "Labels": ownedLabels}) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + t.Setenv("DOCKER_HOST", "tcp://"+strings.TrimPrefix(srv.URL, "http://")) + t.Setenv("DOCKER_TLS_VERIFY", "") + t.Setenv("DOCKER_CERT_PATH", "") + + mgr, err := NewManager(ManagerConfig{Network: "provider-a", IsolateTenantNetworks: true}) + if err != nil { + t.Fatalf("NewManager: %v", err) + } + t.Cleanup(func() { _ = mgr.Close() }) + for _, tc := range []struct { + name string + containerLabels map[string]string + targetLabels map[string]string + want []string + }{ + {name: "owned", containerLabels: map[string]string{"pulse.tenant.id": "t-acme", "pulse.managed": "true"}, targetLabels: ownedLabels, want: []string{"provider-a-tenant-t-acme"}}, + {name: "unlabelled network", containerLabels: map[string]string{"pulse.tenant.id": "t-acme", "pulse.managed": "true"}}, + {name: "wrong network tenant", containerLabels: map[string]string{"pulse.tenant.id": "t-acme", "pulse.managed": "true"}, targetLabels: map[string]string{"pulse.tenant.id": "t-other", tenantRuntimeNetworkLabel: tenantRuntimeNetworkLabelValue}}, + {name: "unmanaged container", containerLabels: map[string]string{"pulse.tenant.id": "t-acme"}, targetLabels: ownedLabels}, + {name: "no container tenant", containerLabels: map[string]string{"pulse.managed": "true"}, targetLabels: ownedLabels}, + } { + t.Run(tc.name, func(t *testing.T) { + containerLabels, targetLabels = tc.containerLabels, tc.targetLabels + got := mgr.tenantNetworkNamesForContainer(context.Background(), "client-a") + if len(got) != len(tc.want) || (len(got) > 0 && got[0] != tc.want[0]) { + t.Fatalf("tenantNetworkNamesForContainer = %v, want %v", got, tc.want) + } + }) + } +} + +func TestTenantNetworkCleanupDisconnectsOnlyAttachedLocalSupport(t *testing.T) { + const target = "provider-a-tenant-t-acme" + const localAttached = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + const localReplacement = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" + const otherAttached = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + var listFilters map[string]map[string]bool + var disconnected []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Api-Version", "1.47") + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/networks/"+target): + _ = json.NewEncoder(w).Encode(map[string]any{ + "Name": target, "Labels": map[string]string{"pulse.tenant.id": "t-acme", tenantRuntimeNetworkLabel: tenantRuntimeNetworkLabelValue}, + "Containers": map[string]any{localAttached: map[string]any{}, otherAttached: map[string]any{}}, + }) + case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/containers/json"): + if err := json.Unmarshal([]byte(r.URL.Query().Get("filters")), &listFilters); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + items := []map[string]string{{"Id": localAttached}, {"Id": localReplacement}} + if !listFilters["network"]["provider-a"] { + items = append(items, map[string]string{"Id": otherAttached}) + } + _ = json.NewEncoder(w).Encode(items) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/networks/"+target+"/disconnect"): + var body struct{ Container string } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + disconnected = append(disconnected, body.Container) + _, _ = w.Write([]byte(`{}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(srv.Close) + t.Setenv("DOCKER_HOST", "tcp://"+strings.TrimPrefix(srv.URL, "http://")) + t.Setenv("DOCKER_TLS_VERIFY", "") + t.Setenv("DOCKER_CERT_PATH", "") + + mgr, err := NewManager(ManagerConfig{Network: "provider-a", IsolateTenantNetworks: true, SupportContainerLabels: []string{providerSupportTraefikLabel}}) + if err != nil { + t.Fatalf("NewManager: %v", err) + } + t.Cleanup(func() { _ = mgr.Close() }) + if err := mgr.disconnectSupportContainersFromTenantNetwork(context.Background(), target); err != nil { + t.Fatalf("disconnectSupportContainersFromTenantNetwork: %v", err) + } + if !listFilters["network"]["provider-a"] || !listFilters["label"][providerSupportTraefikLabel] { + t.Fatalf("support selection did not identify the local provider: %v", listFilters) + } + if len(disconnected) != 1 || disconnected[0] != localAttached { + t.Fatalf("disconnected = %v, want only attached local support %s", disconnected, localAttached) + } +}