mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 20:25:56 +00:00
fix(provider-msp): scope tenant cleanup to owning provider
Only remove a managed client network with the expected name and tenant labels. Select attached support containers by role and provider ingress before forced disconnect, avoiding cross-provider mutations on shared Docker hosts. Update both cloud-paid and deployment-installability contracts with the regression boundary. Change-source: pulse-maintainer
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user