From 5be7a32e6ab0c360ac93a1e0785ef02e06800374 Mon Sep 17 00:00:00 2001 From: Leonhard Schulze Date: Mon, 19 Oct 2026 18:02:57 +0200 Subject: [PATCH 1/6] refactor(cloud): extract public IPs service in CAPG style --- cloud/cleanup.go | 21 ++ cloud/cleanup_test.go | 20 ++ cloud/client.go | 20 +- cloud/errors.go | 33 +- cloud/fake/client.go | 349 +++++++++++++++---- cloud/sdk_client.go | 284 +++------------ cloud/sdk_client_test.go | 19 +- cloud/services/publicip/reconcile.go | 248 ++++++++++++++ cloud/services/publicip/reconcile_test.go | 360 ++++++++++++++++++++ cloud/services/publicip/service.go | 64 ++++ cloud/services/shared/errors.go | 44 +++ cloud/{ => services/shared}/errors_test.go | 2 +- cloud/services/shared/types.go | 28 ++ cloud/services/shared/utils.go | 96 ++++++ cloud/types.go | 28 +- controller/stackitcluster_bastion.go | 44 ++- controller/stackitcluster_infrastructure.go | 14 +- scope/cluster_scope.go | 57 +++- test/e2e/e2e_test.go | 5 +- util/conditions.go | 3 +- 20 files changed, 1364 insertions(+), 375 deletions(-) create mode 100644 cloud/services/publicip/reconcile.go create mode 100644 cloud/services/publicip/reconcile_test.go create mode 100644 cloud/services/publicip/service.go create mode 100644 cloud/services/shared/errors.go rename cloud/{ => services/shared}/errors_test.go (98%) create mode 100644 cloud/services/shared/types.go create mode 100644 cloud/services/shared/utils.go diff --git a/cloud/cleanup.go b/cloud/cleanup.go index 9f155d1..3d635e0 100644 --- a/cloud/cleanup.go +++ b/cloud/cleanup.go @@ -15,6 +15,8 @@ import ( "fmt" "maps" "strings" + + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" ) // CleanupByTags deletes provider-managed cloud resources matching tags without @@ -29,6 +31,25 @@ func CleanupByTags(ctx context.Context, client Client, tags map[string]string) e bastionTags := make(map[string]string, len(tags)+1) maps.Copy(bastionTags, tags) bastionTags["cluster-api-provider-stackit/resource-role"] = "bastion" + bastionServers, err := client.ListServersByTags(ctx, bastionTags) + if err != nil { + errs = append(errs, err.Error()) + } + publicIPs, err := publicip.ListByTags(ctx, client.IaaSClient(), client.ProjectID(), client.Region(), tags) + if err != nil { + errs = append(errs, err.Error()) + } else { + for _, pubIP := range publicIPs { + for _, server := range bastionServers { + if err := publicip.DetachFromServer(ctx, client.IaaSClient(), client.ProjectID(), client.Region(), server.ID, pubIP.ID); err != nil { + errs = append(errs, fmt.Sprintf("detach public IP %q from server %q: %v", pubIP.ID, server.ID, err)) + } + } + if err := publicip.DeleteByID(ctx, client.IaaSClient(), client.ProjectID(), client.Region(), pubIP.ID); err != nil { + errs = append(errs, fmt.Sprintf("delete public IP %q: %v", pubIP.ID, err)) + } + } + } if err := client.DeleteBastion(ctx, BastionInput{Tags: bastionTags}, Bastion{}); err != nil && !IsNotFound(err) { errs = append(errs, fmt.Sprintf("delete bastion resources: %v", err)) } diff --git a/cloud/cleanup_test.go b/cloud/cleanup_test.go index 6ef2778..40f2304 100644 --- a/cloud/cleanup_test.go +++ b/cloud/cleanup_test.go @@ -36,6 +36,12 @@ func TestCleanupByTagsDeletesMatchingCloudResources(t *testing.T) { if _, err := client.CreateServer(ctx, cloud.CreateServerInput{Name: "other", Tags: otherTags}); err != nil { t.Fatalf("CreateServer() error = %v", err) } + if _, err := client.CreatePublicIP(ctx, tags); err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } + if _, err := client.CreatePublicIP(ctx, otherTags); err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } if _, err := client.EnsureAPIServerLoadBalancer(ctx, cloud.LoadBalancerInput{ Name: "matched", Tags: tags, @@ -86,4 +92,18 @@ func TestCleanupByTagsDeletesMatchingCloudResources(t *testing.T) { if len(matchedLoadBalancers) != 0 { t.Fatalf("matched load balancer count = %d, want 0", len(matchedLoadBalancers)) } + matchedPublicIPs, err := client.ListPublicIPsByTags(ctx, tags) + if err != nil { + t.Fatalf("ListPublicIPsByTags() error = %v", err) + } + if len(matchedPublicIPs) != 0 { + t.Fatalf("matched public IP count = %d, want 0", len(matchedPublicIPs)) + } + otherPublicIPs, err := client.ListPublicIPsByTags(ctx, otherTags) + if err != nil { + t.Fatalf("ListPublicIPsByTags(other) error = %v", err) + } + if len(otherPublicIPs) != 1 { + t.Fatalf("other public IP count = %d, want 1", len(otherPublicIPs)) + } } diff --git a/cloud/client.go b/cloud/client.go index a3ab870..7e5bd73 100644 --- a/cloud/client.go +++ b/cloud/client.go @@ -10,7 +10,17 @@ You may obtain a copy of the License at package cloud -import "context" +import ( + "context" + + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" +) + +// Reconciler is a generic interface used by components offering a type of service. +type Reconciler interface { + Reconcile(ctx context.Context) error + Delete(ctx context.Context) error +} // Client is the abstraction over STACKIT cloud APIs used by the controllers. // @@ -23,6 +33,12 @@ import "context" // - be idempotent (CreateServer/EnsureAPIServerLoadBalancer must not produce // duplicates when a resource with matching tags already exists) type Client interface { + ProjectID() string + + Region() string + + IaaSClient() *iaas.APIClient + GetServer(ctx context.Context, id string) (*Server, error) FindServerByTags(ctx context.Context, tags map[string]string) (*Server, error) @@ -43,8 +59,6 @@ type Client interface { DeleteNodeSSHAccess(ctx context.Context, tags map[string]string) error - ListPublicIPsByTags(ctx context.Context, tags map[string]string) ([]*PublicIP, error) - ListSecurityGroupsByTags(ctx context.Context, tags map[string]string) ([]*SecurityGroup, error) EnsureAPIServerLoadBalancer(ctx context.Context, input LoadBalancerInput) (*LoadBalancer, error) diff --git a/cloud/errors.go b/cloud/errors.go index 1aeaa68..e038288 100644 --- a/cloud/errors.go +++ b/cloud/errors.go @@ -10,35 +10,32 @@ You may obtain a copy of the License at package cloud -import "errors" +import "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" -// Sentinel errors returned by Client implementations. Controllers use -// errors.Is to classify them into requeue / condition-update behavior. +// Sentinel errors returned by Client implementations, re-exported from shared. +// Controllers use errors.Is to classify them into requeue / condition-update behavior. var ( - ErrNotFound = errors.New("not found") - ErrUnauthorized = errors.New("unauthorized") - ErrInvalidInput = errors.New("invalid input") - ErrConflict = errors.New("conflict") - ErrTransient = errors.New("transient") + ErrNotFound = shared.ErrNotFound + ErrUnauthorized = shared.ErrUnauthorized + ErrInvalidInput = shared.ErrInvalidInput + ErrConflict = shared.ErrConflict + ErrTransient = shared.ErrTransient ) // IsNotFound reports whether err wraps ErrNotFound. -func IsNotFound(err error) bool { return errors.Is(err, ErrNotFound) } +func IsNotFound(err error) bool { return shared.IsNotFound(err) } // IsUnauthorized reports whether err wraps ErrUnauthorized. -func IsUnauthorized(err error) bool { return errors.Is(err, ErrUnauthorized) } +func IsUnauthorized(err error) bool { return shared.IsUnauthorized(err) } // IsInvalidInput reports whether err wraps ErrInvalidInput. -func IsInvalidInput(err error) bool { return errors.Is(err, ErrInvalidInput) } +func IsInvalidInput(err error) bool { return shared.IsInvalidInput(err) } // IsConflict reports whether err wraps ErrConflict. -func IsConflict(err error) bool { return errors.Is(err, ErrConflict) } +func IsConflict(err error) bool { return shared.IsConflict(err) } // IsTransient reports whether err wraps ErrTransient. -func IsTransient(err error) bool { return errors.Is(err, ErrTransient) } +func IsTransient(err error) bool { return shared.IsTransient(err) } -// IsRetryable reports whether err should trigger a requeue (conflict or -// transient). -func IsRetryable(err error) bool { - return IsConflict(err) || IsTransient(err) -} +// IsRetryable reports whether err should trigger a requeue (conflict or transient). +func IsRetryable(err error) bool { return shared.IsRetryable(err) } diff --git a/cloud/fake/client.go b/cloud/fake/client.go index 06465cc..531ae31 100644 --- a/cloud/fake/client.go +++ b/cloud/fake/client.go @@ -14,12 +14,18 @@ package fake import ( "context" + "encoding/json" "fmt" "maps" + "net/http" + "net/http/httptest" "slices" + "strings" "sync" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + "github.com/stackitcloud/stackit-sdk-go/core/config" + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) const ( @@ -36,24 +42,31 @@ type Client struct { publicIPs map[string]*publicIPEntry securityGroups map[string]*securityGroupEntry networks map[string]*cloud.Network + iaasClient *iaas.APIClient nextID int // failure injection: if non-nil, the next call returns this error and the // field is cleared. - FailNextCreateServer error - FailNextDeleteServer error - FailNextGetServer error - FailNextFindServer error - FailNextEnsureLB error - FailNextDeleteLB error - FailNextSetTargets error - FailNextGetNetwork error - FailNextEnsureBastion error - FailNextDeleteBastion error - FailNextEnsureNodeSSH error - FailNextDeleteNodeSSH error - FailNextListAZ error + FailNextCreateServer error + FailNextDeleteServer error + FailNextGetServer error + FailNextFindServer error + FailNextEnsureLB error + FailNextDeleteLB error + FailNextSetTargets error + FailNextGetNetwork error + FailNextEnsureBastion error + FailNextDeleteBastion error + FailNextEnsureNodeSSH error + FailNextDeleteNodeSSH error + FailNextListAZ error + FailNextGetPublicIP error + FailNextListPublicIPs error + FailNextCreatePublicIP error + FailNextDeletePublicIP error + FailNextAddPublicIP error + FailNextRemovePublicIP error // AvailabilityZones is returned by ListAvailabilityZones if non-empty. // Defaults to eu01-1, eu01-2, eu01-3. @@ -113,9 +126,30 @@ func New(networks ...cloud.Network) *Client { n := networks[i] c.networks[n.ID] = &n } + iaasClient, err := iaas.NewAPIClient( + config.WithoutAuthentication(), + config.WithHTTPClient(&http.Client{Transport: roundTripperFunc(c.roundTripIaaS)}), + config.WithEndpoint("http://stackit.fake.local"), + ) + if err != nil { + panic(fmt.Sprintf("create fake iaas client: %v", err)) + } + c.iaasClient = iaasClient return c } +func (c *Client) ProjectID() string { + return "00000000-0000-0000-0000-000000000000" +} + +func (c *Client) Region() string { + return "eu01" +} + +func (c *Client) IaaSClient() *iaas.APIClient { + return c.iaasClient +} + func consume(p *error) error { if *p == nil { return nil @@ -127,7 +161,7 @@ func consume(p *error) error { func (c *Client) genID() string { c.nextID++ - return fmt.Sprintf("fake-%d", c.nextID) + return fmt.Sprintf("00000000-0000-4000-8000-%012d", c.nextID) } func (c *Client) GetServer(_ context.Context, id string) (*cloud.Server, error) { @@ -260,14 +294,11 @@ func (c *Client) EnsureBastion(_ context.Context, input cloud.BastionInput) (*cl } for _, serverEntry := range c.servers { if len(input.Tags) > 0 && mapContains(serverEntry.tags, input.Tags) { - publicIP := c.publicIPByTags(input.Tags) securityGroup := c.securityGroupByTags(input.Tags) return &cloud.Bastion{ ServerID: serverEntry.server.ID, ServerState: serverEntry.server.State, ServerPowerStatus: serverEntry.server.PowerStatus, - PublicIPID: idOfPublicIP(publicIP), - PublicIP: ipOfPublicIP(publicIP), SecurityGroupID: idOfSecurityGroup(securityGroup), }, nil } @@ -300,20 +331,10 @@ func (c *Client) EnsureBastion(_ context.Context, input cloud.BastionInput) (*cl userData: append([]byte(nil), input.CloudInit...), } - publicIPID := c.genID() - publicIP := &cloud.PublicIP{ - ID: publicIPID, - IP: "203.0.113.22", - NetworkInterfaceID: "fake-nic-" + serverID, - } - c.publicIPs[publicIPID] = &publicIPEntry{publicIP: publicIP, tags: copyTags(input.Tags)} - c.EnsureBastionCalls++ return &cloud.Bastion{ ServerID: serverID, ServerState: server.State, - PublicIPID: publicIPID, - PublicIP: publicIP.IP, SecurityGroupID: securityGroupID, }, nil } @@ -328,9 +349,6 @@ func (c *Client) DeleteBastion(_ context.Context, input cloud.BastionInput, stat if status.ServerID != "" { delete(c.servers, status.ServerID) } - if status.PublicIPID != "" { - delete(c.publicIPs, status.PublicIPID) - } if status.SecurityGroupID != "" { delete(c.securityGroups, status.SecurityGroupID) } @@ -339,11 +357,6 @@ func (c *Client) DeleteBastion(_ context.Context, input cloud.BastionInput, stat delete(c.servers, id) } } - for id, entry := range c.publicIPs { - if mapContains(entry.tags, input.Tags) { - delete(c.publicIPs, id) - } - } for id, entry := range c.securityGroups { if mapContains(entry.tags, input.Tags) { delete(c.securityGroups, id) @@ -407,10 +420,31 @@ func (c *Client) DeleteNodeSSHAccess(_ context.Context, tags map[string]string) return nil } +func (c *Client) GetPublicIP(_ context.Context, id string) (*cloud.PublicIP, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if err := consume(&c.FailNextGetPublicIP); err != nil { + return nil, err + } + if id == "" { + return nil, fmt.Errorf("public IP ID is empty: %w", cloud.ErrInvalidInput) + } + entry, ok := c.publicIPs[id] + if !ok { + return nil, fmt.Errorf("public IP %q: %w", id, cloud.ErrNotFound) + } + out := *entry.publicIP + return &out, nil +} + func (c *Client) ListPublicIPsByTags(_ context.Context, tags map[string]string) ([]*cloud.PublicIP, error) { c.mu.Lock() defer c.mu.Unlock() + if err := consume(&c.FailNextListPublicIPs); err != nil { + return nil, err + } if len(tags) == 0 { return nil, fmt.Errorf("empty tags: %w", cloud.ErrInvalidInput) } @@ -424,6 +458,82 @@ func (c *Client) ListPublicIPsByTags(_ context.Context, tags map[string]string) return publicIPs, nil } +func (c *Client) CreatePublicIP(_ context.Context, tags map[string]string) (*cloud.PublicIP, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if err := consume(&c.FailNextCreatePublicIP); err != nil { + return nil, err + } + id := c.genID() + publicIP := &cloud.PublicIP{ + ID: id, + IP: "203.0.113.22", + } + c.publicIPs[id] = &publicIPEntry{ + publicIP: publicIP, + tags: copyTags(tags), + } + out := *publicIP + return &out, nil +} + +func (c *Client) DeletePublicIP(_ context.Context, id string) error { + c.mu.Lock() + defer c.mu.Unlock() + + if err := consume(&c.FailNextDeletePublicIP); err != nil { + return err + } + if id == "" { + return fmt.Errorf("public IP ID is empty: %w", cloud.ErrInvalidInput) + } + if _, ok := c.publicIPs[id]; !ok { + return fmt.Errorf("public IP %q: %w", id, cloud.ErrNotFound) + } + delete(c.publicIPs, id) + return nil +} + +func (c *Client) AddPublicIPToServer(_ context.Context, serverID, publicIPID string) error { + c.mu.Lock() + defer c.mu.Unlock() + + if err := consume(&c.FailNextAddPublicIP); err != nil { + return err + } + if serverID == "" || publicIPID == "" { + return fmt.Errorf("server ID and public IP ID are required: %w", cloud.ErrInvalidInput) + } + if _, ok := c.servers[serverID]; !ok { + return fmt.Errorf("server %q: %w", serverID, cloud.ErrNotFound) + } + entry, ok := c.publicIPs[publicIPID] + if !ok { + return fmt.Errorf("public IP %q: %w", publicIPID, cloud.ErrNotFound) + } + entry.publicIP.NetworkInterfaceID = "fake-nic-" + serverID + return nil +} + +func (c *Client) RemovePublicIPFromServer(_ context.Context, serverID, publicIPID string) error { + c.mu.Lock() + defer c.mu.Unlock() + + if err := consume(&c.FailNextRemovePublicIP); err != nil { + return err + } + if serverID == "" || publicIPID == "" { + return fmt.Errorf("server ID and public IP ID are required: %w", cloud.ErrInvalidInput) + } + entry, ok := c.publicIPs[publicIPID] + if !ok { + return fmt.Errorf("public IP %q: %w", publicIPID, cloud.ErrNotFound) + } + entry.publicIP.NetworkInterfaceID = "" + return nil +} + func (c *Client) ListSecurityGroupsByTags(_ context.Context, tags map[string]string) ([]*cloud.SecurityGroup, error) { c.mu.Lock() defer c.mu.Unlock() @@ -684,16 +794,6 @@ func containsString(values []string, needle string) bool { return slices.Contains(values, needle) } -func (c *Client) publicIPByTags(tags map[string]string) *cloud.PublicIP { - for _, entry := range c.publicIPs { - if mapContains(entry.tags, tags) { - out := *entry.publicIP - return &out - } - } - return nil -} - func (c *Client) securityGroupByTags(tags map[string]string) *cloud.SecurityGroup { for _, entry := range c.securityGroups { if mapContains(entry.tags, tags) { @@ -704,20 +804,6 @@ func (c *Client) securityGroupByTags(tags map[string]string) *cloud.SecurityGrou return nil } -func idOfPublicIP(publicIP *cloud.PublicIP) string { - if publicIP == nil { - return "" - } - return publicIP.ID -} - -func ipOfPublicIP(publicIP *cloud.PublicIP) string { - if publicIP == nil { - return "" - } - return publicIP.IP -} - func idOfSecurityGroup(securityGroup *cloud.SecurityGroup) string { if securityGroup == nil { return "" @@ -745,3 +831,148 @@ func cloneServer(s *cloud.Server) *cloud.Server { } return &out } + +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { + return f(r) +} + +func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { + rec := httptest.NewRecorder() + rec.Header().Set("Content-Type", "application/json") + path := r.URL.Path + + switch { + case strings.Contains(path, "/servers/") && strings.Contains(path, "/public-ips/"): + parts := strings.Split(path, "/servers/") + sub := strings.Split(parts[1], "/public-ips/") + serverID, publicIPID := sub[0], sub[1] + var err error + switch r.Method { + case http.MethodPut: + err = c.AddPublicIPToServer(r.Context(), serverID, publicIPID) + case http.MethodDelete: + err = c.RemovePublicIPFromServer(r.Context(), serverID, publicIPID) + default: + err = cloud.ErrInvalidInput + } + if err != nil { + writeFakeError(rec, err) + } else { + rec.WriteHeader(http.StatusNoContent) + } + + case r.Method == http.MethodPost && strings.HasSuffix(path, "/public-ips"): + var body struct { + Labels map[string]string `json:"labels"` + } + if r.Body != nil { + _ = json.NewDecoder(r.Body).Decode(&body) + } + created, err := c.CreatePublicIP(r.Context(), body.Labels) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(fakePublicIPMap(created, body.Labels)) + } + + case r.Method == http.MethodGet && strings.HasSuffix(path, "/public-ips"): + tags := parseLabelSelector(r.URL.Query().Get("label_selector")) + items, err := c.listPublicIPItems(tags) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(map[string]any{"items": items}) + } + + case r.Method == http.MethodGet && strings.Contains(path, "/public-ips/"): + id := path[strings.LastIndex(path, "/public-ips/")+len("/public-ips/"):] + got, err := c.GetPublicIP(r.Context(), id) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(fakePublicIPMap(got, nil)) + } + + case r.Method == http.MethodDelete && strings.Contains(path, "/public-ips/"): + id := path[strings.LastIndex(path, "/public-ips/")+len("/public-ips/"):] + if err := c.DeletePublicIP(r.Context(), id); err != nil { + writeFakeError(rec, err) + } else { + rec.WriteHeader(http.StatusNoContent) + } + + default: + rec.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(rec).Encode(map[string]any{"message": "not found"}) + } + + return rec.Result(), nil +} + +func (c *Client) listPublicIPItems(tags map[string]string) ([]map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if err := consume(&c.FailNextListPublicIPs); err != nil { + return nil, err + } + if len(tags) == 0 { + return nil, fmt.Errorf("empty tags: %w", cloud.ErrInvalidInput) + } + items := make([]map[string]any, 0, len(c.publicIPs)) + for _, entry := range c.publicIPs { + if mapContains(entry.tags, tags) { + items = append(items, fakePublicIPMap(entry.publicIP, entry.tags)) + } + } + return items, nil +} + +func fakePublicIPMap(p *cloud.PublicIP, tags map[string]string) map[string]any { + m := map[string]any{ + "id": p.ID, + "ip": p.IP, + } + if p.NetworkInterfaceID != "" { + m["networkInterface"] = p.NetworkInterfaceID + } + if len(tags) > 0 { + labels := make(map[string]any, len(tags)) + for k, v := range tags { + labels[k] = v + } + m["labels"] = labels + } + return m +} + +func parseLabelSelector(selector string) map[string]string { + if selector == "" { + return nil + } + tags := make(map[string]string) + for part := range strings.SplitSeq(selector, ",") { + if k, v, ok := strings.Cut(part, "="); ok && k != "" { + tags[k] = v + } + } + return tags +} + +func writeFakeError(rec *httptest.ResponseRecorder, err error) { + status := http.StatusInternalServerError + switch { + case cloud.IsNotFound(err): + status = http.StatusNotFound + case cloud.IsConflict(err): + status = http.StatusConflict + case cloud.IsInvalidInput(err): + status = http.StatusBadRequest + case cloud.IsUnauthorized(err): + status = http.StatusUnauthorized + } + rec.WriteHeader(status) + _ = json.NewEncoder(rec).Encode(map[string]any{"message": err.Error()}) +} diff --git a/cloud/sdk_client.go b/cloud/sdk_client.go index 433e22f..0fa1751 100644 --- a/cloud/sdk_client.go +++ b/cloud/sdk_client.go @@ -14,15 +14,13 @@ import ( "cmp" "context" "encoding/base64" - "errors" "fmt" - "net/http" "os" "slices" "strings" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" "github.com/stackitcloud/stackit-sdk-go/core/config" - "github.com/stackitcloud/stackit-sdk-go/core/oapierror" iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" lb "github.com/stackitcloud/stackit-sdk-go/services/loadbalancer/v2api" ) @@ -83,6 +81,18 @@ func NewClient(_ context.Context, creds Credentials) (Client, error) { }, nil } +func (c *SDKClient) ProjectID() string { + return c.projectID +} + +func (c *SDKClient) Region() string { + return c.region +} + +func (c *SDKClient) IaaSClient() *iaas.APIClient { + return c.iaasClient +} + func stackitOptions(creds Credentials, endpoint string) []config.ConfigurationOption { opts := []config.ConfigurationOption{ config.WithUserAgent("cluster-api-provider-stackit"), @@ -104,7 +114,7 @@ func stackitOptions(creds Credentials, endpoint string) []config.ConfigurationOp func (c *SDKClient) GetServer(ctx context.Context, id string) (*Server, error) { server, err := c.iaasClient.DefaultAPI.GetServer(ctx, c.projectID, c.region, id).Execute() if err != nil { - return nil, classifySDKError("get server", err) + return nil, shared.ClassifySDKError("get server", err) } return c.serverFromSDK(ctx, server), nil } @@ -128,15 +138,15 @@ func (c *SDKClient) ListServersByTags(ctx context.Context, tags map[string]strin return nil, fmt.Errorf("%w: empty tag selector", ErrInvalidInput) } resp, err := c.iaasClient.DefaultAPI.ListServers(ctx, c.projectID, c.region). - LabelSelector(labelSelector(tags)). + LabelSelector(shared.LabelSelector(tags)). Details(true). Execute() if err != nil { - return nil, classifySDKError("list servers", err) + return nil, shared.ClassifySDKError("list servers", err) } matched := []*Server{} for _, server := range resp.GetItems() { - if labelsContainTags(server.GetLabels(), tags) { + if shared.LabelsContainTags(server.GetLabels(), tags) { server := server matched = append(matched, c.serverFromSDK(ctx, &server)) } @@ -159,7 +169,7 @@ func (c *SDKClient) CreateServer(ctx context.Context, input CreateServerInput) ( payload.SetImageId(input.ImageID) } if len(input.Tags) > 0 { - payload.SetLabels(tagsToSDKLabels(input.Tags)) + payload.SetLabels(shared.TagsToSDKLabels(input.Tags)) } networking := iaas.NewCreateServerNetworking() if input.NetworkID != "" { @@ -198,7 +208,7 @@ func (c *SDKClient) CreateServer(ctx context.Context, input CreateServerInput) ( CreateServerPayload(*payload). Execute() if err != nil { - return nil, classifySDKError("create server", err) + return nil, shared.ClassifySDKError("create server", err) } return c.serverFromSDK(ctx, server), nil } @@ -206,7 +216,7 @@ func (c *SDKClient) CreateServer(ctx context.Context, input CreateServerInput) ( func (c *SDKClient) ListAvailabilityZones(ctx context.Context) ([]string, error) { resp, err := c.iaasClient.DefaultAPI.ListAvailabilityZones(ctx, c.region).Execute() if err != nil { - return nil, classifySDKError("list availability zones", err) + return nil, shared.ClassifySDKError("list availability zones", err) } if resp == nil || resp.Items == nil { return nil, nil @@ -216,7 +226,7 @@ func (c *SDKClient) ListAvailabilityZones(ctx context.Context) ([]string, error) func (c *SDKClient) DeleteServer(ctx context.Context, id string) error { if err := c.iaasClient.DefaultAPI.DeleteServer(ctx, c.projectID, c.region, id).Execute(); err != nil { - return classifySDKError("delete server", err) + return shared.ClassifySDKError("delete server", err) } return nil } @@ -224,7 +234,7 @@ func (c *SDKClient) DeleteServer(ctx context.Context, id string) error { func (c *SDKClient) GetNetwork(ctx context.Context, id string) (*Network, error) { network, err := c.iaasClient.DefaultAPI.GetNetwork(ctx, c.projectID, c.region, id).Execute() if err != nil { - return nil, classifySDKError("get network", err) + return nil, shared.ClassifySDKError("get network", err) } ipv4 := network.GetIpv4() return &Network{ @@ -283,31 +293,10 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas return nil, err } - publicIP, err := c.ensurePublicIP(ctx, input.Tags) - if err != nil { - return nil, err - } - if publicIP.NetworkInterfaceID == "" { - if err := c.iaasClient.DefaultAPI. - AddPublicIpToServer(ctx, c.projectID, c.region, server.ID, publicIP.ID). - Execute(); err != nil { - err := classifySDKError("add public IP to server", err) - if !IsConflict(err) { - return nil, err - } - } - publicIP, err = c.getPublicIP(ctx, publicIP.ID) - if err != nil { - return nil, err - } - } - return &Bastion{ ServerID: server.ID, ServerState: server.State, ServerPowerStatus: server.PowerStatus, - PublicIPID: publicIP.ID, - PublicIP: publicIP.IP, SecurityGroupID: securityGroup.ID, }, nil } @@ -323,16 +312,6 @@ func (c *SDKClient) DeleteBastion(ctx context.Context, input BastionInput, statu } } - publicIPID := status.PublicIPID - if publicIPID == "" { - publicIP, err := c.findPublicIPByTags(ctx, input.Tags) - if err == nil { - publicIPID = publicIP.ID - } else if !IsNotFound(err) { - return err - } - } - securityGroupID := status.SecurityGroupID if securityGroupID == "" { securityGroup, err := c.findSecurityGroupByTags(ctx, input.Tags) @@ -343,21 +322,11 @@ func (c *SDKClient) DeleteBastion(ctx context.Context, input BastionInput, statu } } - if serverID != "" && publicIPID != "" { - if err := c.iaasClient.DefaultAPI. - RemovePublicIpFromServer(ctx, c.projectID, c.region, serverID, publicIPID). - Execute(); err != nil { - err := classifySDKError("remove public IP from server", err) - if !IsNotFound(err) && !IsConflict(err) { - return err - } - } - } if serverID != "" && securityGroupID != "" { if err := c.iaasClient.DefaultAPI. RemoveSecurityGroupFromServer(ctx, c.projectID, c.region, serverID, securityGroupID). Execute(); err != nil { - err := classifySDKError("remove security group from server", err) + err := shared.ClassifySDKError("remove security group from server", err) if !IsNotFound(err) && !IsConflict(err) { return err } @@ -368,14 +337,6 @@ func (c *SDKClient) DeleteBastion(ctx context.Context, input BastionInput, statu return err } } - if publicIPID != "" { - if err := c.iaasClient.DefaultAPI.DeletePublicIP(ctx, c.projectID, c.region, publicIPID).Execute(); err != nil { - err := classifySDKError("delete public IP", err) - if !IsNotFound(err) { - return err - } - } - } if securityGroupID != "" { if err := c.deleteSecurityGroupRules(ctx, securityGroupID); err != nil { return err @@ -383,7 +344,7 @@ func (c *SDKClient) DeleteBastion(ctx context.Context, input BastionInput, statu if err := c.iaasClient.DefaultAPI. DeleteSecurityGroup(ctx, c.projectID, c.region, securityGroupID). Execute(); err != nil { - err := classifySDKError("delete security group", err) + err := shared.ClassifySDKError("delete security group", err) if !IsNotFound(err) { return err } @@ -432,7 +393,7 @@ func (c *SDKClient) DeleteNodeSSHAccess(ctx context.Context, tags map[string]str if err := c.iaasClient.DefaultAPI. RemoveSecurityGroupFromServer(ctx, c.projectID, c.region, server.ID, securityGroup.ID). Execute(); err != nil { - err := classifySDKError("remove security group from server", err) + err := shared.ClassifySDKError("remove security group from server", err) if !IsNotFound(err) && !IsConflict(err) { return err } @@ -444,7 +405,7 @@ func (c *SDKClient) DeleteNodeSSHAccess(ctx context.Context, tags map[string]str if err := c.iaasClient.DefaultAPI. DeleteSecurityGroup(ctx, c.projectID, c.region, securityGroup.ID). Execute(); err != nil { - err := classifySDKError("delete security group", err) + err := shared.ClassifySDKError("delete security group", err) if !IsNotFound(err) { return err } @@ -452,39 +413,19 @@ func (c *SDKClient) DeleteNodeSSHAccess(ctx context.Context, tags map[string]str return nil } -func (c *SDKClient) ListPublicIPsByTags(ctx context.Context, tags map[string]string) ([]*PublicIP, error) { - if len(tags) == 0 { - return nil, fmt.Errorf("%w: empty tag selector", ErrInvalidInput) - } - resp, err := c.iaasClient.DefaultAPI.ListPublicIPs(ctx, c.projectID, c.region). - LabelSelector(labelSelector(tags)). - Execute() - if err != nil { - return nil, classifySDKError("list public IPs", err) - } - matched := []*PublicIP{} - for _, publicIP := range resp.GetItems() { - if labelsContainTags(publicIP.GetLabels(), tags) { - publicIP := publicIP - matched = append(matched, publicIPFromSDK(&publicIP)) - } - } - return matched, nil -} - func (c *SDKClient) ListSecurityGroupsByTags(ctx context.Context, tags map[string]string) ([]*SecurityGroup, error) { if len(tags) == 0 { return nil, fmt.Errorf("%w: empty tag selector", ErrInvalidInput) } resp, err := c.iaasClient.DefaultAPI.ListSecurityGroups(ctx, c.projectID, c.region). - LabelSelector(labelSelector(tags)). + LabelSelector(shared.LabelSelector(tags)). Execute() if err != nil { - return nil, classifySDKError("list security groups", err) + return nil, shared.ClassifySDKError("list security groups", err) } matched := []*SecurityGroup{} for _, securityGroup := range resp.GetItems() { - if labelsContainTags(securityGroup.GetLabels(), tags) { + if shared.LabelsContainTags(securityGroup.GetLabels(), tags) { securityGroup := securityGroup matched = append(matched, securityGroupFromSDK(&securityGroup)) } @@ -502,7 +443,7 @@ func (c *SDKClient) EnsureAPIServerLoadBalancer(ctx context.Context, input LoadB payload.SetName(input.Name) payload.SetRegion(input.Region) if len(input.Tags) > 0 { - payload.SetLabels(loadBalancerLabels(input.Tags)) + payload.SetLabels(shared.LoadBalancerLabels(input.Tags)) } options := lb.NewLoadBalancerOptions() options.SetEphemeralAddress(true) @@ -544,14 +485,14 @@ func (c *SDKClient) EnsureAPIServerLoadBalancer(ctx context.Context, input LoadB CreateLoadBalancerPayload(*payload). Execute() if err != nil { - return nil, classifySDKError("create load balancer", err) + return nil, shared.ClassifySDKError("create load balancer", err) } return loadBalancerFromSDK(lbResp), nil } func (c *SDKClient) DeleteAPIServerLoadBalancer(ctx context.Context, id string) error { if _, err := c.lbClient.DefaultAPI.DeleteLoadBalancer(ctx, c.projectID, c.region, id).Execute(); err != nil { - return classifySDKError("delete load balancer", err) + return shared.ClassifySDKError("delete load balancer", err) } return nil } @@ -565,16 +506,16 @@ func (c *SDKClient) ListAPIServerLoadBalancersByTags( } resp, err := c.lbClient.DefaultAPI.ListLoadBalancers(ctx, c.projectID, c.region).Execute() if err != nil { - err := classifySDKError("list load balancers", err) + err := shared.ClassifySDKError("list load balancers", err) if isLoadBalancerServiceNotEnabled(err) { return nil, nil } return nil, err } matched := []*LoadBalancer{} - labels := loadBalancerLabels(tags) + labels := shared.LoadBalancerLabels(tags) for _, candidate := range resp.GetLoadBalancers() { - if mapContains(candidate.GetLabels(), labels) { + if shared.MapContains(candidate.GetLabels(), labels) { candidate := candidate matched = append(matched, loadBalancerFromSDK(&candidate)) } @@ -608,7 +549,7 @@ func (c *SDKClient) SetAPIServerLoadBalancerTargets( loadBalancer, err := c.lbClient.DefaultAPI.GetLoadBalancer(ctx, c.projectID, c.region, loadBalancerID).Execute() if err != nil { - return classifySDKError("get load balancer", err) + return shared.ClassifySDKError("get load balancer", err) } targetPool := apiServerTargetPool(loadBalancer) if targetPool == nil { @@ -670,12 +611,12 @@ func (c *SDKClient) ensureBastionSecurityGroup(ctx context.Context, input Bastio } payload := iaas.NewCreateSecurityGroupPayload(input.Name + "-ssh") payload.SetDescription("Cluster API Provider STACKIT bastion SSH access") - payload.SetLabels(tagsToSDKLabels(input.Tags)) + payload.SetLabels(shared.TagsToSDKLabels(input.Tags)) securityGroup, err := c.iaasClient.DefaultAPI.CreateSecurityGroup(ctx, c.projectID, c.region). CreateSecurityGroupPayload(*payload). Execute() if err != nil { - return nil, classifySDKError("create security group", err) + return nil, shared.ClassifySDKError("create security group", err) } return securityGroupFromSDK(securityGroup), nil } @@ -688,12 +629,12 @@ func (c *SDKClient) ensureNodeSSHSecurityGroup(ctx context.Context, input NodeSS } payload := iaas.NewCreateSecurityGroupPayload(input.Name) payload.SetDescription("Cluster API Provider STACKIT node SSH access from bastion") - payload.SetLabels(tagsToSDKLabels(input.Tags)) + payload.SetLabels(shared.TagsToSDKLabels(input.Tags)) securityGroup, err := c.iaasClient.DefaultAPI.CreateSecurityGroup(ctx, c.projectID, c.region). CreateSecurityGroupPayload(*payload). Execute() if err != nil { - return nil, classifySDKError("create security group", err) + return nil, shared.ClassifySDKError("create security group", err) } return securityGroupFromSDK(securityGroup), nil } @@ -704,7 +645,7 @@ func (c *SDKClient) ensureBastionSecurityGroupRules(ctx context.Context, securit } resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, securityGroupID).Execute() if err != nil { - return classifySDKError("list security group rules", err) + return shared.ClassifySDKError("list security group rules", err) } existingRules := resp.GetItems() desired := make(map[string]struct{}, len(cidrs)) @@ -731,7 +672,7 @@ func (c *SDKClient) ensureBastionSecurityGroupRules(ctx context.Context, securit if _, err := c.iaasClient.DefaultAPI.CreateSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID). CreateSecurityGroupRulePayload(*rule). Execute(); err != nil { - return classifySDKError("create security group rule", err) + return shared.ClassifySDKError("create security group rule", err) } } @@ -752,7 +693,7 @@ func (c *SDKClient) ensureBastionSecurityGroupRules(ctx context.Context, securit if err := c.iaasClient.DefaultAPI. DeleteSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID, ruleID). Execute(); err != nil { - err := classifySDKError("delete security group rule", err) + err := shared.ClassifySDKError("delete security group rule", err) if !IsNotFound(err) { return err } @@ -770,7 +711,7 @@ func (c *SDKClient) ensureNodeSSHSecurityGroupRule( } resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, nodeSecurityGroupID).Execute() if err != nil { - return classifySDKError("list security group rules", err) + return shared.ClassifySDKError("list security group rules", err) } if hasRemoteSecurityGroupSSHRule(resp.GetItems(), bastionSecurityGroupID) { return nil @@ -783,50 +724,11 @@ func (c *SDKClient) ensureNodeSSHSecurityGroupRule( if _, err := c.iaasClient.DefaultAPI.CreateSecurityGroupRule(ctx, c.projectID, c.region, nodeSecurityGroupID). CreateSecurityGroupRulePayload(*rule). Execute(); err != nil { - return classifySDKError("create security group rule", err) + return shared.ClassifySDKError("create security group rule", err) } return nil } -func (c *SDKClient) ensurePublicIP(ctx context.Context, tags map[string]string) (*PublicIP, error) { - if existing, err := c.findPublicIPByTags(ctx, tags); err == nil { - return existing, nil - } else if !IsNotFound(err) { - return nil, err - } - payload := iaas.NewCreatePublicIPPayload() - payload.SetLabels(tagsToSDKLabels(tags)) - publicIP, err := c.iaasClient.DefaultAPI.CreatePublicIP(ctx, c.projectID, c.region). - CreatePublicIPPayload(*payload). - Execute() - if err != nil { - return nil, classifySDKError("create public IP", err) - } - return publicIPFromSDK(publicIP), nil -} - -func (c *SDKClient) getPublicIP(ctx context.Context, id string) (*PublicIP, error) { - publicIP, err := c.iaasClient.DefaultAPI.GetPublicIP(ctx, c.projectID, c.region, id).Execute() - if err != nil { - return nil, classifySDKError("get public IP", err) - } - return publicIPFromSDK(publicIP), nil -} - -func (c *SDKClient) findPublicIPByTags(ctx context.Context, tags map[string]string) (*PublicIP, error) { - matched, err := c.ListPublicIPsByTags(ctx, tags) - if err != nil { - return nil, err - } - if len(matched) == 0 { - return nil, fmt.Errorf("%w: no public IP matching tags", ErrNotFound) - } - if len(matched) > 1 { - return nil, fmt.Errorf("%w: multiple public IPs matching tags", ErrConflict) - } - return matched[0], nil -} - func (c *SDKClient) findSecurityGroupByTags(ctx context.Context, tags map[string]string) (*SecurityGroup, error) { matched, err := c.ListSecurityGroupsByTags(ctx, tags) if err != nil { @@ -851,7 +753,7 @@ func (c *SDKClient) addSecurityGroupToServer(ctx context.Context, serverID, secu if err := c.iaasClient.DefaultAPI. AddSecurityGroupToServer(ctx, c.projectID, c.region, serverID, securityGroupID). Execute(); err != nil { - err := classifySDKError("add security group to server", err) + err := shared.ClassifySDKError("add security group to server", err) if !IsConflict(err) && !IsInvalidInput(err) && !IsNotFound(err) { return err } @@ -862,7 +764,7 @@ func (c *SDKClient) addSecurityGroupToServer(ctx context.Context, serverID, secu func (c *SDKClient) deleteSecurityGroupRules(ctx context.Context, securityGroupID string) error { resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, securityGroupID).Execute() if err != nil { - err := classifySDKError("list security group rules", err) + err := shared.ClassifySDKError("list security group rules", err) if IsNotFound(err) { return nil } @@ -876,7 +778,7 @@ func (c *SDKClient) deleteSecurityGroupRules(ctx context.Context, securityGroupI if err := c.iaasClient.DefaultAPI. DeleteSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID, ruleID). Execute(); err != nil { - err := classifySDKError("delete security group rule", err) + err := shared.ClassifySDKError("delete security group rule", err) if !IsNotFound(err) { return err } @@ -943,7 +845,7 @@ func (c *SDKClient) updateAPIServerTargetPool( UpdateTargetPool(ctx, c.projectID, c.region, loadBalancerID, apiserverTargetPoolName). UpdateTargetPoolPayload(*payload). Execute(); err != nil { - return classifySDKError("update load balancer target pool", err) + return shared.ClassifySDKError("update load balancer target pool", err) } return nil } @@ -992,22 +894,11 @@ func loadBalancerFromSDK(in *lb.LoadBalancer) *LoadBalancer { return &LoadBalancer{ ID: in.GetName(), Name: in.GetName(), - IP: firstNonEmpty(in.GetExternalAddress(), in.GetPrivateAddress()), + IP: shared.FirstNonEmpty(in.GetExternalAddress(), in.GetPrivateAddress()), Port: listenerPort(in.GetListeners()), } } -func publicIPFromSDK(in *iaas.PublicIp) *PublicIP { - if in == nil { - return nil - } - return &PublicIP{ - ID: in.GetId(), - IP: in.GetIp(), - NetworkInterfaceID: in.GetNetworkInterface(), - } -} - func securityGroupFromSDK(in *iaas.SecurityGroup) *SecurityGroup { if in == nil { return nil @@ -1027,32 +918,6 @@ func listenerPort(listeners []lb.Listener) int32 { return 0 } -func labelSelector(tags map[string]string) string { - parts := make([]string, 0, len(tags)) - for k, v := range tags { - parts = append(parts, k+"="+v) - } - return strings.Join(parts, ",") -} - -func labelsContainTags(labels map[string]any, tags map[string]string) bool { - for k, want := range tags { - got, ok := labels[k] - if !ok || fmt.Sprint(got) != want { - return false - } - } - return true -} - -func tagsToSDKLabels(tags map[string]string) map[string]any { - out := make(map[string]any, len(tags)) - for k, v := range tags { - out[k] = v - } - return out -} - func clusterServerSelectorFromTags(tags map[string]string) map[string]string { selector := map[string]string{} for _, key := range []string{ @@ -1067,55 +932,6 @@ func clusterServerSelectorFromTags(tags map[string]string) map[string]string { return selector } -func mapContains(haystack, needle map[string]string) bool { - for k, v := range needle { - if haystack[k] != v { - return false - } - } - return true -} - -func loadBalancerLabels(tags map[string]string) map[string]string { - out := make(map[string]string, len(tags)) - for k, v := range tags { - out[strings.ReplaceAll(k, "/", ".")] = v - } - return out -} - -func firstNonEmpty(values ...string) string { - for _, value := range values { - if value != "" { - return value - } - } - return "" -} - -func classifySDKError(op string, err error) error { - var oapiErr *oapierror.GenericOpenAPIError - if errors.As(err, &oapiErr) { - switch oapiErr.StatusCode { - case http.StatusNotFound: - return fmt.Errorf("%w: %s: %v", ErrNotFound, op, err) - case http.StatusUnauthorized, http.StatusForbidden: - return fmt.Errorf("%w: %s: %v", ErrUnauthorized, op, err) - case http.StatusConflict: - return fmt.Errorf("%w: %s: %v", ErrConflict, op, err) - case http.StatusBadRequest, http.StatusUnprocessableEntity: - return fmt.Errorf("%w: %s: %v", ErrInvalidInput, op, err) - case http.StatusTooManyRequests: - return fmt.Errorf("%w: %s: %v", ErrTransient, op, err) - default: - if oapiErr.StatusCode >= 500 { - return fmt.Errorf("%w: %s: %v", ErrTransient, op, err) - } - } - } - return fmt.Errorf("%w: %s: %v", ErrTransient, op, err) -} - func isLoadBalancerServiceNotEnabled(err error) bool { return IsUnauthorized(err) && strings.Contains(err.Error(), "Service not enabled") } diff --git a/cloud/sdk_client_test.go b/cloud/sdk_client_test.go index 34a3e9b..ad0c9e7 100644 --- a/cloud/sdk_client_test.go +++ b/cloud/sdk_client_test.go @@ -469,9 +469,8 @@ func lookup(m map[string]any, key string) any { // short-circuits on an existing server, making this the only re-attach path. func TestSDKClientEnsureBastionToleratesDuplicateSecurityGroupAttach(t *testing.T) { var ( - createPayload map[string]any - attachCallCount int - publicIPAssigned bool + createPayload map[string]any + attachCallCount int ) server := newSDKTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path := r.URL.Path @@ -502,13 +501,6 @@ func TestSDKClientEnsureBastionToleratesDuplicateSecurityGroupAttach(t *testing. "code": 400, "msg": "request invalid: Invalid input for security_groups. Reason: Duplicate items in the list.", }) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/public-ips"): - writeJSON(t, w, map[string]any{"items": []any{}}) - case r.Method == http.MethodPost && strings.HasSuffix(path, "/public-ips"): - writeJSON(t, w, map[string]any{"id": "66666666-6666-4666-8666-666666666666", "ip": "203.0.113.10"}) - case strings.Contains(path, "/public-ips/"): - publicIPAssigned = true - writeJSON(t, w, map[string]any{"id": "66666666-6666-4666-8666-666666666666", "ip": "203.0.113.10"}) case r.Method == http.MethodGet && strings.Contains(path, "/servers/"): writeJSON(t, w, map[string]any{ "id": testSDKServerID, "name": "bastion", "status": "ACTIVE", "machineType": "c2i.1", @@ -541,11 +533,8 @@ func TestSDKClientEnsureBastionToleratesDuplicateSecurityGroupAttach(t *testing. if len(groups) != 1 || groups[0] != testSDKSecurityGroup { t.Fatalf("create payload securityGroups = %v, want [%s]", createPayload["securityGroups"], testSDKSecurityGroup) } - if !publicIPAssigned { - t.Fatal("public IP was never assigned to the bastion server") - } - if bastion.PublicIP != "203.0.113.10" { - t.Fatalf("bastion.PublicIP = %q, want 203.0.113.10", bastion.PublicIP) + if bastion.ServerID != testSDKServerID { + t.Fatalf("bastion.ServerID = %q, want %s", bastion.ServerID, testSDKServerID) } } diff --git a/cloud/services/publicip/reconcile.go b/cloud/services/publicip/reconcile.go new file mode 100644 index 0000000..d3f947c --- /dev/null +++ b/cloud/services/publicip/reconcile.go @@ -0,0 +1,248 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package publicip + +import ( + "context" + "fmt" + + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" +) + +// Reconcile reconciles cluster public IP resources. +func (s *Service) Reconcile(ctx context.Context) error { + log := logf.FromContext(ctx) + if !s.scope.BastionSpec().Enabled { + return s.Delete(ctx) + } + + log.V(1).Info("Reconciling public IP resources") + publicIP, err := s.createOrGetPublicIP(ctx) + if err != nil { + return err + } + + serverID := s.scope.BastionStatus().ServerID + if serverID != "" && publicIP.NetworkInterfaceID == "" { + log.V(1).Info("Attaching public IP to server", "publicIPID", publicIP.ID, "serverID", serverID) + if err := s.iaasClient.DefaultAPI. + AddPublicIpToServer(ctx, s.scope.ProjectID(), s.scope.Region(), serverID, publicIP.ID). + Execute(); err != nil { + err := shared.ClassifySDKError("add public IP to server", err) + if !shared.IsConflict(err) { + log.Error(err, "Failed to attach public IP to server", "publicIPID", publicIP.ID, "serverID", serverID) + return err + } + } + publicIP, err = s.getPublicIP(ctx, publicIP.ID) + if err != nil { + log.Error(err, "Failed to get public IP after attachment", "publicIPID", publicIP.ID) + return err + } + } + + s.scope.SetBastionPublicIP(publicIP.ID, publicIP.IP) + return nil +} + +// Delete deletes cluster public IP resources. +func (s *Service) Delete(ctx context.Context) error { + log := logf.FromContext(ctx) + log.V(1).Info("Deleting public IP resources") + + publicIP, err := s.findPublicIP(ctx) + if err != nil { + return err + } + if publicIP == nil { + s.scope.SetBastionPublicIP("", "") + return nil + } + + serverID, err := s.resolveServerID(ctx) + if err != nil { + return err + } + if serverID != "" { + log.V(1).Info("Detaching public IP from server", "publicIPID", publicIP.ID, "serverID", serverID) + if err := DetachFromServer(ctx, s.iaasClient, s.scope.ProjectID(), s.scope.Region(), serverID, publicIP.ID); err != nil { + log.Error(err, "Failed to detach public IP from server", "publicIPID", publicIP.ID, "serverID", serverID) + return err + } + } + + log.V(1).Info("Deleting public IP", "publicIPID", publicIP.ID) + if err := DeleteByID(ctx, s.iaasClient, s.scope.ProjectID(), s.scope.Region(), publicIP.ID); err != nil { + log.Error(err, "Failed to delete public IP", "publicIPID", publicIP.ID) + return err + } + + s.scope.SetBastionPublicIP("", "") + return nil +} + +func (s *Service) createOrGetPublicIP(ctx context.Context) (*PublicIP, error) { + log := logf.FromContext(ctx) + + publicIP, err := s.findPublicIP(ctx) + if err != nil { + return nil, err + } + if publicIP != nil { + return publicIP, nil + } + + log.V(1).Info("Creating public IP") + payload := iaas.NewCreatePublicIPPayload() + if tags := s.scope.BastionTags(); len(tags) > 0 { + payload.SetLabels(shared.TagsToSDKLabels(tags)) + } + created, err := s.iaasClient.DefaultAPI. + CreatePublicIP(ctx, s.scope.ProjectID(), s.scope.Region()). + CreatePublicIPPayload(*payload). + Execute() + if err != nil { + err := shared.ClassifySDKError("create public IP", err) + log.Error(err, "Failed to create public IP") + return nil, err + } + return publicIPFromSDK(created), nil +} + +func (s *Service) findPublicIP(ctx context.Context) (*PublicIP, error) { + log := logf.FromContext(ctx) + if id := s.scope.BastionStatus().PublicIPID; id != "" { + publicIP, err := s.getPublicIP(ctx, id) + if err == nil { + return publicIP, nil + } + if !shared.IsNotFound(err) { + log.Error(err, "Failed to get public IP", "publicIPID", id) + return nil, err + } + } + + matched, err := ListByTags(ctx, s.iaasClient, s.scope.ProjectID(), s.scope.Region(), s.scope.BastionTags()) + if err != nil { + log.Error(err, "Failed to list public IPs by tags") + return nil, err + } + if len(matched) == 0 { + return nil, nil + } + if len(matched) > 1 { + return nil, fmt.Errorf("%w: multiple public IPs matching tags", shared.ErrConflict) + } + return matched[0], nil +} + +func (s *Service) getPublicIP(ctx context.Context, id string) (*PublicIP, error) { + if id == "" { + return nil, fmt.Errorf("%w: public IP ID is empty", shared.ErrInvalidInput) + } + publicIP, err := s.iaasClient.DefaultAPI.GetPublicIP(ctx, s.scope.ProjectID(), s.scope.Region(), id).Execute() + if err != nil { + return nil, shared.ClassifySDKError("get public IP", err) + } + return publicIPFromSDK(publicIP), nil +} + +func (s *Service) resolveServerID(ctx context.Context) (string, error) { + if id := s.scope.BastionStatus().ServerID; id != "" { + return id, nil + } + server, err := s.client.FindServerByTags(ctx, s.scope.BastionTags()) + if err != nil { + if shared.IsNotFound(err) { + return "", nil + } + return "", err + } + return server.ID, nil +} + +// ListByTags lists STACKIT public IPs matching the given tag selector. +func ListByTags( + ctx context.Context, + iaasClient *iaas.APIClient, + projectID, region string, + tags map[string]string, +) ([]*PublicIP, error) { + if len(tags) == 0 { + return nil, fmt.Errorf("%w: empty tag selector", shared.ErrInvalidInput) + } + resp, err := iaasClient.DefaultAPI.ListPublicIPs(ctx, projectID, region). + LabelSelector(shared.LabelSelector(tags)). + Execute() + if err != nil { + return nil, shared.ClassifySDKError("list public IPs", err) + } + matched := []*PublicIP{} + for _, publicIP := range resp.GetItems() { + if shared.LabelsContainTags(publicIP.GetLabels(), tags) { + publicIP := publicIP + matched = append(matched, publicIPFromSDK(&publicIP)) + } + } + return matched, nil +} + +// DetachFromServer detaches a public IP from a server, ignoring NotFound and Conflict errors. +func DetachFromServer( + ctx context.Context, + iaasClient *iaas.APIClient, + projectID, region, serverID, publicIPID string, +) error { + if serverID == "" || publicIPID == "" { + return fmt.Errorf("%w: server ID and public IP ID are required", shared.ErrInvalidInput) + } + if err := iaasClient.DefaultAPI. + RemovePublicIpFromServer(ctx, projectID, region, serverID, publicIPID). + Execute(); err != nil { + err := shared.ClassifySDKError("remove public IP from server", err) + if !shared.IsNotFound(err) && !shared.IsConflict(err) { + return err + } + } + return nil +} + +// DeleteByID deletes a public IP by ID, ignoring NotFound errors. +func DeleteByID( + ctx context.Context, + iaasClient *iaas.APIClient, + projectID, region, id string, +) error { + if id == "" { + return fmt.Errorf("%w: public IP ID is empty", shared.ErrInvalidInput) + } + if err := iaasClient.DefaultAPI.DeletePublicIP(ctx, projectID, region, id).Execute(); err != nil { + err := shared.ClassifySDKError("delete public IP", err) + if !shared.IsNotFound(err) { + return err + } + } + return nil +} + +func publicIPFromSDK(in *iaas.PublicIp) *PublicIP { + if in == nil { + return nil + } + return &PublicIP{ + ID: in.GetId(), + IP: in.GetIp(), + NetworkInterfaceID: in.GetNetworkInterface(), + } +} diff --git a/cloud/services/publicip/reconcile_test.go b/cloud/services/publicip/reconcile_test.go new file mode 100644 index 0000000..b162e5d --- /dev/null +++ b/cloud/services/publicip/reconcile_test.go @@ -0,0 +1,360 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package publicip_test + +import ( + "context" + "errors" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + cloudfake "github.com/stackitcloud/cluster-api-provider-stackit/cloud/fake" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" + "github.com/stackitcloud/cluster-api-provider-stackit/scope" +) + +func init() { + _ = clusterv1.AddToScheme(scheme.Scheme) + _ = infrav1.AddToScheme(scheme.Scheme) +} + +func newTestClusterScope(t *testing.T, bastionEnabled bool, cloudClient cloud.Client) *scope.ClusterScope { + t.Helper() + + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + } + stackitCluster := &infrav1.StackitCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: infrav1.StackitClusterSpec{ + ProjectID: "00000000-0000-0000-0000-000000000000", + Region: "eu01", + Bastion: infrav1.StackitBastionSpec{ + Enabled: bastionEnabled, + }, + }, + } + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + WithObjects(cluster, stackitCluster). + Build() + + clusterScope, err := scope.NewClusterScope(k8sClient, cluster, stackitCluster) + if err != nil { + t.Fatalf("NewClusterScope() error = %v", err) + } + clusterScope.SetCloudClient(cloudClient) + return clusterScope +} + +func TestServiceReconcile(t *testing.T) { + t.Run("deletes existing public IP when bastion is disabled", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, false, fakeCloud) + + created, err := fakeCloud.CreatePublicIP(ctx, clusterScope.BastionTags()) + if err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } + clusterScope.SetBastionPublicIP(created.ID, created.IP) + + svc := publicip.New(clusterScope) + if err := svc.Reconcile(ctx); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + + if fakeCloud.PublicIPCount() != 0 { + t.Fatalf("PublicIPCount() = %d, want 0", fakeCloud.PublicIPCount()) + } + if clusterScope.BastionStatus().PublicIPID != "" { + t.Fatalf("BastionStatus().PublicIPID = %q, want empty", clusterScope.BastionStatus().PublicIPID) + } + }) + + t.Run("creates public IP and attaches it to the bastion server", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + + server, err := fakeCloud.CreateServer(ctx, cloud.CreateServerInput{ + Name: "test-cluster-bastion", + Tags: clusterScope.BastionTags(), + }) + if err != nil { + t.Fatalf("CreateServer() error = %v", err) + } + clusterScope.StackitCluster.Status.Bastion.ServerID = server.ID + + svc := publicip.New(clusterScope) + if err := svc.Reconcile(ctx); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + + if fakeCloud.PublicIPCount() != 1 { + t.Fatalf("PublicIPCount() = %d, want 1", fakeCloud.PublicIPCount()) + } + if clusterScope.BastionStatus().PublicIPID == "" { + t.Fatal("BastionStatus().PublicIPID is empty, want set") + } + if clusterScope.StackitCluster.Status.Bastion.PublicIP != "203.0.113.22" { + t.Fatalf("Bastion.PublicIP = %q, want 203.0.113.22", clusterScope.StackitCluster.Status.Bastion.PublicIP) + } + + gotIP, err := fakeCloud.GetPublicIP(ctx, clusterScope.BastionStatus().PublicIPID) + if err != nil { + t.Fatalf("GetPublicIP() error = %v", err) + } + if gotIP.NetworkInterfaceID == "" { + t.Fatal("PublicIP NetworkInterfaceID is empty, want attached to server") + } + }) + + t.Run("reuses existing public IP found by status ID without listing by tags", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + + server, err := fakeCloud.CreateServer(ctx, cloud.CreateServerInput{ + Name: "test-cluster-bastion", + Tags: clusterScope.BastionTags(), + }) + if err != nil { + t.Fatalf("CreateServer() error = %v", err) + } + clusterScope.StackitCluster.Status.Bastion.ServerID = server.ID + + existing, err := fakeCloud.CreatePublicIP(ctx, clusterScope.BastionTags()) + if err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } + clusterScope.SetBastionPublicIP(existing.ID, existing.IP) + fakeCloud.FailNextListPublicIPs = shared.ErrTransient + + svc := publicip.New(clusterScope) + if err := svc.Reconcile(ctx); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + + if fakeCloud.PublicIPCount() != 1 { + t.Fatalf("PublicIPCount() = %d, want 1", fakeCloud.PublicIPCount()) + } + if clusterScope.BastionStatus().PublicIPID != existing.ID { + t.Fatalf("BastionStatus().PublicIPID = %q, want %q", clusterScope.BastionStatus().PublicIPID, existing.ID) + } + }) + + t.Run("reuses existing public IP found by tags without creating duplicate", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + + server, err := fakeCloud.CreateServer(ctx, cloud.CreateServerInput{ + Name: "test-cluster-bastion", + Tags: clusterScope.BastionTags(), + }) + if err != nil { + t.Fatalf("CreateServer() error = %v", err) + } + clusterScope.StackitCluster.Status.Bastion.ServerID = server.ID + + existing, err := fakeCloud.CreatePublicIP(ctx, clusterScope.BastionTags()) + if err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } + + svc := publicip.New(clusterScope) + if err := svc.Reconcile(ctx); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + + if fakeCloud.PublicIPCount() != 1 { + t.Fatalf("PublicIPCount() = %d, want 1", fakeCloud.PublicIPCount()) + } + if clusterScope.BastionStatus().PublicIPID != existing.ID { + t.Fatalf("BastionStatus().PublicIPID = %q, want %q", clusterScope.BastionStatus().PublicIPID, existing.ID) + } + }) + + t.Run("tolerates conflict when attaching public IP to server", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + + server, err := fakeCloud.CreateServer(ctx, cloud.CreateServerInput{ + Name: "test-cluster-bastion", + Tags: clusterScope.BastionTags(), + }) + if err != nil { + t.Fatalf("CreateServer() error = %v", err) + } + clusterScope.StackitCluster.Status.Bastion.ServerID = server.ID + fakeCloud.FailNextAddPublicIP = shared.ErrConflict + + svc := publicip.New(clusterScope) + if err := svc.Reconcile(ctx); err != nil { + t.Fatalf("Reconcile() error = %v, want nil on conflict", err) + } + if clusterScope.BastionStatus().PublicIPID == "" { + t.Fatal("BastionStatus().PublicIPID is empty, want set") + } + }) + + t.Run("returns conflict error when multiple public IPs match tags", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + + if _, err := fakeCloud.CreatePublicIP(ctx, clusterScope.BastionTags()); err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } + if _, err := fakeCloud.CreatePublicIP(ctx, clusterScope.BastionTags()); err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } + + svc := publicip.New(clusterScope) + err := svc.Reconcile(ctx) + if !errors.Is(err, shared.ErrConflict) { + t.Fatalf("Reconcile() error = %v, want ErrConflict", err) + } + }) + + t.Run("returns error when creating public IP fails", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + fakeCloud.FailNextCreatePublicIP = shared.ErrTransient + + svc := publicip.New(clusterScope) + err := svc.Reconcile(ctx) + if !errors.Is(err, shared.ErrTransient) { + t.Fatalf("Reconcile() error = %v, want ErrTransient", err) + } + }) +} + +func TestServiceDelete(t *testing.T) { + t.Run("detaches and deletes public IP using recorded status ID", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + + server, err := fakeCloud.CreateServer(ctx, cloud.CreateServerInput{ + Name: "test-cluster-bastion", + Tags: clusterScope.BastionTags(), + }) + if err != nil { + t.Fatalf("CreateServer() error = %v", err) + } + clusterScope.StackitCluster.Status.Bastion.ServerID = server.ID + + created, err := fakeCloud.CreatePublicIP(ctx, clusterScope.BastionTags()) + if err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } + if err := fakeCloud.AddPublicIPToServer(ctx, server.ID, created.ID); err != nil { + t.Fatalf("AddPublicIPToServer() error = %v", err) + } + clusterScope.SetBastionPublicIP(created.ID, created.IP) + + svc := publicip.New(clusterScope) + if err := svc.Delete(ctx); err != nil { + t.Fatalf("Delete() error = %v", err) + } + + if fakeCloud.PublicIPCount() != 0 { + t.Fatalf("PublicIPCount() = %d, want 0", fakeCloud.PublicIPCount()) + } + if clusterScope.BastionStatus().PublicIPID != "" { + t.Fatalf("BastionStatus().PublicIPID = %q, want empty", clusterScope.BastionStatus().PublicIPID) + } + if clusterScope.StackitCluster.Status.Bastion.PublicIP != "" { + t.Fatalf("Bastion.PublicIP = %q, want empty", clusterScope.StackitCluster.Status.Bastion.PublicIP) + } + }) + + t.Run("falls back to tag lookup for both public IP and server when status IDs are empty", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + + server, err := fakeCloud.CreateServer(ctx, cloud.CreateServerInput{ + Name: "test-cluster-bastion", + Tags: clusterScope.BastionTags(), + }) + if err != nil { + t.Fatalf("CreateServer() error = %v", err) + } + created, err := fakeCloud.CreatePublicIP(ctx, clusterScope.BastionTags()) + if err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } + if err := fakeCloud.AddPublicIPToServer(ctx, server.ID, created.ID); err != nil { + t.Fatalf("AddPublicIPToServer() error = %v", err) + } + + svc := publicip.New(clusterScope) + if err := svc.Delete(ctx); err != nil { + t.Fatalf("Delete() error = %v", err) + } + + if fakeCloud.PublicIPCount() != 0 { + t.Fatalf("PublicIPCount() = %d, want 0", fakeCloud.PublicIPCount()) + } + }) + + t.Run("succeeds when no public IP exists", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + + svc := publicip.New(clusterScope) + if err := svc.Delete(ctx); err != nil { + t.Fatalf("Delete() error = %v, want nil", err) + } + }) + + t.Run("returns error when DeletePublicIP fails with non-NotFound error", func(t *testing.T) { + ctx := context.Background() + fakeCloud := cloudfake.New() + clusterScope := newTestClusterScope(t, true, fakeCloud) + + created, err := fakeCloud.CreatePublicIP(ctx, clusterScope.BastionTags()) + if err != nil { + t.Fatalf("CreatePublicIP() error = %v", err) + } + clusterScope.SetBastionPublicIP(created.ID, created.IP) + fakeCloud.FailNextDeletePublicIP = shared.ErrTransient + + svc := publicip.New(clusterScope) + err = svc.Delete(ctx) + if !errors.Is(err, shared.ErrTransient) { + t.Fatalf("Delete() error = %v, want ErrTransient", err) + } + if clusterScope.BastionStatus().PublicIPID != created.ID { + t.Fatalf("BastionStatus().PublicIPID = %q, want preserved %q on error", clusterScope.BastionStatus().PublicIPID, created.ID) + } + }) +} diff --git a/cloud/services/publicip/service.go b/cloud/services/publicip/service.go new file mode 100644 index 0000000..62cc0c0 --- /dev/null +++ b/cloud/services/publicip/service.go @@ -0,0 +1,64 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package publicip + +import ( + "context" + + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + + infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" +) + +// PublicIP describes a STACKIT public IP resource. +type PublicIP struct { + ID string + IP string + NetworkInterfaceID string +} + +// Client defines the cloud API operations required by the public IP service. +type Client interface { + FindServerByTags(ctx context.Context, tags map[string]string) (*shared.Server, error) +} + +// Scope is an interface that provides cluster scope data needed for public IP reconciliation. +type Scope[C Client] interface { + scopeData + Cloud() C +} + +type scopeData interface { + ProjectID() string + Region() string + IaaSClient() *iaas.APIClient + BastionSpec() infrav1.StackitBastionSpec + BastionStatus() infrav1.StackitBastionStatus + BastionTags() map[string]string + SetBastionPublicIP(id, ip string) +} + +// Service implements public IPs reconciler. +type Service struct { + scope scopeData + client Client + iaasClient *iaas.APIClient +} + +// New returns Service from given scope. +func New[C Client](scope Scope[C]) *Service { + return &Service{ + scope: scope, + client: scope.Cloud(), + iaasClient: scope.IaaSClient(), + } +} diff --git a/cloud/services/shared/errors.go b/cloud/services/shared/errors.go new file mode 100644 index 0000000..73ecf9b --- /dev/null +++ b/cloud/services/shared/errors.go @@ -0,0 +1,44 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package shared + +import "errors" + +// Sentinel errors returned by Client implementations. Controllers use +// errors.Is to classify them into requeue / condition-update behavior. +var ( + ErrNotFound = errors.New("not found") + ErrUnauthorized = errors.New("unauthorized") + ErrInvalidInput = errors.New("invalid input") + ErrConflict = errors.New("conflict") + ErrTransient = errors.New("transient") +) + +// IsNotFound reports whether err wraps ErrNotFound. +func IsNotFound(err error) bool { return errors.Is(err, ErrNotFound) } + +// IsUnauthorized reports whether err wraps ErrUnauthorized. +func IsUnauthorized(err error) bool { return errors.Is(err, ErrUnauthorized) } + +// IsInvalidInput reports whether err wraps ErrInvalidInput. +func IsInvalidInput(err error) bool { return errors.Is(err, ErrInvalidInput) } + +// IsConflict reports whether err wraps ErrConflict. +func IsConflict(err error) bool { return errors.Is(err, ErrConflict) } + +// IsTransient reports whether err wraps ErrTransient. +func IsTransient(err error) bool { return errors.Is(err, ErrTransient) } + +// IsRetryable reports whether err should trigger a requeue (conflict or +// transient). +func IsRetryable(err error) bool { + return IsConflict(err) || IsTransient(err) +} diff --git a/cloud/errors_test.go b/cloud/services/shared/errors_test.go similarity index 98% rename from cloud/errors_test.go rename to cloud/services/shared/errors_test.go index e1b82f3..314eb25 100644 --- a/cloud/errors_test.go +++ b/cloud/services/shared/errors_test.go @@ -8,7 +8,7 @@ You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 */ -package cloud +package shared import ( "fmt" diff --git a/cloud/services/shared/types.go b/cloud/services/shared/types.go new file mode 100644 index 0000000..a413a30 --- /dev/null +++ b/cloud/services/shared/types.go @@ -0,0 +1,28 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package shared + +// Server describes a STACKIT compute instance in provider-neutral terms. +type Server struct { + ID string + Name string + State string + PowerStatus string + ProviderID string + AvailabilityZone string + Addresses []Address +} + +// Address is an IP or DNS endpoint of a Server. +type Address struct { + Type string + Address string +} diff --git a/cloud/services/shared/utils.go b/cloud/services/shared/utils.go new file mode 100644 index 0000000..430d98b --- /dev/null +++ b/cloud/services/shared/utils.go @@ -0,0 +1,96 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package shared + +import ( + "errors" + "fmt" + "net/http" + "strings" + + "github.com/stackitcloud/stackit-sdk-go/core/oapierror" +) + +func MapContains(haystack, needle map[string]string) bool { + for k, v := range needle { + if haystack[k] != v { + return false + } + } + return true +} + +// TODO: Move to LB Service +func LoadBalancerLabels(tags map[string]string) map[string]string { + out := make(map[string]string, len(tags)) + for k, v := range tags { + out[strings.ReplaceAll(k, "/", ".")] = v + } + return out +} + +func FirstNonEmpty(values ...string) string { + for _, value := range values { + if value != "" { + return value + } + } + return "" +} + +func LabelSelector(tags map[string]string) string { + parts := make([]string, 0, len(tags)) + for k, v := range tags { + parts = append(parts, k+"="+v) + } + return strings.Join(parts, ",") +} + +func LabelsContainTags(labels map[string]any, tags map[string]string) bool { + for k, want := range tags { + got, ok := labels[k] + if !ok || fmt.Sprint(got) != want { + return false + } + } + return true +} + +func TagsToSDKLabels(tags map[string]string) map[string]any { + out := make(map[string]any, len(tags)) + for k, v := range tags { + out[k] = v + } + return out +} + +func ClassifySDKError(op string, err error) error { + var oapiErr *oapierror.GenericOpenAPIError + if errors.As(err, &oapiErr) { + switch oapiErr.StatusCode { + case http.StatusNotFound: + return fmt.Errorf("%w: %s: %v", ErrNotFound, op, err) + case http.StatusUnauthorized, http.StatusForbidden: + return fmt.Errorf("%w: %s: %v", ErrUnauthorized, op, err) + case http.StatusConflict: + return fmt.Errorf("%w: %s: %v", ErrConflict, op, err) + case http.StatusBadRequest, http.StatusUnprocessableEntity: + return fmt.Errorf("%w: %s: %v", ErrInvalidInput, op, err) + case http.StatusTooManyRequests: + return fmt.Errorf("%w: %s: %v", ErrTransient, op, err) + default: + if oapiErr.StatusCode >= 500 { + return fmt.Errorf("%w: %s: %v", ErrTransient, op, err) + } + } + } + return fmt.Errorf("%w: %s: %v", ErrTransient, op, err) +} diff --git a/cloud/types.go b/cloud/types.go index 289353f..92a63dc 100644 --- a/cloud/types.go +++ b/cloud/types.go @@ -13,22 +13,16 @@ You may obtain a copy of the License at // through the Client interface in this package. package cloud +import ( + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" +) + // Server describes a STACKIT compute instance in provider-neutral terms. -type Server struct { - ID string - Name string - State string - PowerStatus string - ProviderID string - AvailabilityZone string - Addresses []Address -} +type Server = shared.Server // Address is an IP or DNS endpoint of a Server. -type Address struct { - Type string - Address string -} +type Address = shared.Address // Network is an existing STACKIT virtual network referenced by the provider. type Network struct { @@ -47,11 +41,7 @@ type LoadBalancer struct { } // PublicIP describes a STACKIT public IP resource. -type PublicIP struct { - ID string - IP string - NetworkInterfaceID string -} +type PublicIP = publicip.PublicIP // SecurityGroup describes a STACKIT security group. type SecurityGroup struct { @@ -64,8 +54,6 @@ type Bastion struct { ServerID string ServerState string ServerPowerStatus string - PublicIPID string - PublicIP string SecurityGroupID string } diff --git a/controller/stackitcluster_bastion.go b/controller/stackitcluster_bastion.go index e8c3d8b..e2c1a1a 100644 --- a/controller/stackitcluster_bastion.go +++ b/controller/stackitcluster_bastion.go @@ -29,6 +29,7 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" "github.com/stackitcloud/cluster-api-provider-stackit/scope" ) @@ -46,8 +47,6 @@ func (r *StackitClusterReconciler) reconcileBastion( input := bastionservice.Input(stackitCluster, nil) status := cloud.Bastion{ ServerID: stackitCluster.Status.Bastion.ServerID, - PublicIPID: stackitCluster.Status.Bastion.PublicIPID, - PublicIP: stackitCluster.Status.Bastion.PublicIP, SecurityGroupID: stackitCluster.Status.Bastion.SecurityGroupID, } @@ -57,10 +56,7 @@ func (r *StackitClusterReconciler) reconcileBastion( // the status keeps the tag-based cleanup to once per cluster. condition := meta.FindStatusCondition(stackitCluster.Status.Conditions, infrav1.ClusterBastionReadyCondition) if condition == nil || condition.Reason != bastionDisabledReason { - if err := cloudClient.DeleteNodeSSHAccess(ctx, bastionservice.NodeSSHAccessTags(stackitCluster)); err != nil { - return ctrl.Result{}, false, err - } - if err := cloudClient.DeleteBastion(ctx, input, status); err != nil { + if err := r.deleteBastionResources(ctx, cloudClient, clusterScope, input, status, false); err != nil { return ctrl.Result{}, false, err } clusterScope.ClearBastionStatus() @@ -102,10 +98,7 @@ func (r *StackitClusterReconciler) reconcileBastion( input.CloudInit = cloudInit if bastionNeedsRecreate(stackitCluster, cloudInit) { - if err := cloudClient.DeleteNodeSSHAccess(ctx, bastionservice.NodeSSHAccessTags(stackitCluster)); err != nil && !cloud.IsNotFound(err) { - return ctrl.Result{}, false, err - } - if err := cloudClient.DeleteBastion(ctx, input, status); err != nil && !cloud.IsNotFound(err) { + if err := r.deleteBastionResources(ctx, cloudClient, clusterScope, input, status, true); err != nil { return ctrl.Result{}, false, err } clusterScope.ClearBastionStatus() @@ -133,6 +126,9 @@ func (r *StackitClusterReconciler) reconcileBastion( stackitCluster, "BastionCreated", "Created bastion %s", bastion.ServerID, ) } + if err := publicip.New(clusterScope).Reconcile(ctx); err != nil { + return ctrl.Result{}, false, err + } if ready, reason, message, warn := serverStateCondition(bastion.ServerState, bastion.ServerPowerStatus); !ready { previousReason := "" if previous := meta.FindStatusCondition(stackitCluster.Status.Conditions, infrav1.ClusterBastionReadyCondition); previous != nil { @@ -145,7 +141,7 @@ func (r *StackitClusterReconciler) reconcileBastion( } return ctrl.Result{RequeueAfter: 15 * time.Second}, false, nil } - if bastion.PublicIP == "" { + if stackitCluster.Status.Bastion.PublicIP == "" { clusterScope.SetNotReady( "Provisioning", "waiting for bastion public IP address", @@ -188,6 +184,32 @@ func hasBastionStatus(status infrav1.StackitBastionStatus) bool { return status.ServerID != "" || status.PublicIPID != "" || status.PublicIP != "" || status.SecurityGroupID != "" } +func (r *StackitClusterReconciler) deleteBastionResources( + ctx context.Context, + cloudClient cloud.Client, + clusterScope *scope.ClusterScope, + input cloud.BastionInput, + status cloud.Bastion, + ignoreNotFound bool, +) error { + if err := cloudClient.DeleteNodeSSHAccess(ctx, bastionservice.NodeSSHAccessTags(clusterScope.StackitCluster)); err != nil { + if !ignoreNotFound || !cloud.IsNotFound(err) { + return err + } + } + if err := publicip.New(clusterScope).Delete(ctx); err != nil { + if !ignoreNotFound || !cloud.IsNotFound(err) { + return err + } + } + if err := cloudClient.DeleteBastion(ctx, input, status); err != nil { + if !ignoreNotFound || !cloud.IsNotFound(err) { + return err + } + } + return nil +} + func bastionNeedsRecreate(stackitCluster *infrav1.StackitCluster, cloudInit []byte) bool { if !hasBastionStatus(stackitCluster.Status.Bastion) { return false diff --git a/controller/stackitcluster_infrastructure.go b/controller/stackitcluster_infrastructure.go index f4c4f8e..f2c55e8 100644 --- a/controller/stackitcluster_infrastructure.go +++ b/controller/stackitcluster_infrastructure.go @@ -33,6 +33,7 @@ import ( "github.com/stackitcloud/cluster-api-provider-stackit/cloud" bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" loadbalancerservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/loadbalancer" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" "github.com/stackitcloud/cluster-api-provider-stackit/scope" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -66,6 +67,7 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS infrav1.ClusterReadyCondition, ) } + clusterScope.SetCloudClient(cloudClient) clusterScope.SetConditions( metav1.ConditionTrue, "Available", @@ -412,6 +414,7 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS ) return ctrl.Result{}, err } + clusterScope.SetCloudClient(cloudClient) loadBalancerID, err := loadbalancerservice.ResolveID(ctx, cloudClient, stackitCluster) if err != nil { log.Error(err, "Failed to resolve API server load balancer for deletion", "cluster", stackitCluster.Name) @@ -438,10 +441,15 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS "Failed to delete bastion node SSH access: %v", err) return ctrl.Result{}, err } + hadBastion := hasBastionStatus(stackitCluster.Status.Bastion) + if err := publicip.New(clusterScope).Delete(ctx); err != nil && !cloud.IsNotFound(err) { + log.Error(err, "Failed to delete bastion public IP", "cluster", stackitCluster.Name) + record.Warnf(stackitCluster, "FailedDeleteBastion", + "Failed to delete bastion public IP: %v", err) + return ctrl.Result{}, err + } if err := cloudClient.DeleteBastion(ctx, bastionservice.Input(stackitCluster, nil), cloud.Bastion{ ServerID: stackitCluster.Status.Bastion.ServerID, - PublicIPID: stackitCluster.Status.Bastion.PublicIPID, - PublicIP: stackitCluster.Status.Bastion.PublicIP, SecurityGroupID: stackitCluster.Status.Bastion.SecurityGroupID, }); err != nil && !cloud.IsNotFound(err) { log.Error(err, "Failed to delete bastion", "cluster", stackitCluster.Name) @@ -449,7 +457,7 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS "Failed to delete bastion: %v", err) return ctrl.Result{}, err } - if hasBastionStatus(stackitCluster.Status.Bastion) { + if hadBastion { clusterScope.ClearBastionStatus() record.Event(stackitCluster, "BastionDeleted", "Deleted bastion") } diff --git a/scope/cluster_scope.go b/scope/cluster_scope.go index d1a4705..12fc45a 100644 --- a/scope/cluster_scope.go +++ b/scope/cluster_scope.go @@ -25,6 +25,7 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" "github.com/stackitcloud/cluster-api-provider-stackit/util" + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) // ClusterScope holds the per-reconcile state for a StackitCluster. @@ -32,6 +33,7 @@ type ClusterScope struct { Client client.Client Cluster *clusterv1.Cluster StackitCluster *infrav1.StackitCluster + CloudClient cloud.Client patchHelper *patch.Helper } @@ -87,13 +89,9 @@ func (s *ClusterScope) SetAPIServerEndpoint(endpoint clusterv1.APIEndpoint) { } func (s *ClusterScope) SetBastionStatus(bastion *cloud.Bastion, cloudInitHash string) { - s.StackitCluster.Status.Bastion = infrav1.StackitBastionStatus{ - ServerID: bastion.ServerID, - PublicIPID: bastion.PublicIPID, - PublicIP: bastion.PublicIP, - SecurityGroupID: bastion.SecurityGroupID, - CloudInitHash: cloudInitHash, - } + s.StackitCluster.Status.Bastion.ServerID = bastion.ServerID + s.StackitCluster.Status.Bastion.SecurityGroupID = bastion.SecurityGroupID + s.StackitCluster.Status.Bastion.CloudInitHash = cloudInitHash } func (s *ClusterScope) ClearBastionStatus() { @@ -103,3 +101,48 @@ func (s *ClusterScope) ClearBastionStatus() { func (s *ClusterScope) SetFailureDomains(failureDomains []clusterv1.FailureDomain) { s.StackitCluster.Status.FailureDomains = failureDomains } + +func (s *ClusterScope) Cloud() cloud.Client { + return s.CloudClient +} + +func (s *ClusterScope) SetCloudClient(cloudClient cloud.Client) { + s.CloudClient = cloudClient +} + +func (s *ClusterScope) ProjectID() string { + if s.StackitCluster.Spec.ProjectID != "" { + return s.StackitCluster.Spec.ProjectID + } + return s.CloudClient.ProjectID() +} + +func (s *ClusterScope) Region() string { + if s.StackitCluster.Spec.Region != "" { + return s.StackitCluster.Spec.Region + } + return s.CloudClient.Region() +} + +func (s *ClusterScope) IaaSClient() *iaas.APIClient { + return s.CloudClient.IaaSClient() +} + +func (s *ClusterScope) BastionSpec() infrav1.StackitBastionSpec { + return s.StackitCluster.Spec.Bastion +} + +func (s *ClusterScope) BastionStatus() infrav1.StackitBastionStatus { + return s.StackitCluster.Status.Bastion +} + +func (s *ClusterScope) BastionTags() map[string]string { + tags := util.ClusterTags(s.StackitCluster.Name, s.StackitCluster.Namespace, s.StackitCluster.Spec.AdditionalLabels) + tags[util.LabelResourceRole] = util.ResourceRoleBastion + return tags +} + +func (s *ClusterScope) SetBastionPublicIP(id, ip string) { + s.StackitCluster.Status.Bastion.PublicIPID = id + s.StackitCluster.Status.Bastion.PublicIP = ip +} diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 224d56a..d753b39 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -34,6 +34,7 @@ import ( . "github.com/onsi/gomega" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" "github.com/stackitcloud/cluster-api-provider-stackit/test/utils" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -490,7 +491,7 @@ var _ = Describe("Manager", Ordered, func() { Eventually(func(g Gomega) { output := kubectlOutput(g, "get", "stackitcluster", clusterName, "-n", cfg.Namespace, "-o", "jsonpath={.status.bastion.publicIP}") g.Expect(output).NotTo(BeEmpty()) - publicIPs, err := cloudClient.ListPublicIPsByTags(ctx, stackitE2EBastionTags(testID)) + publicIPs, err := publicip.ListByTags(ctx, cloudClient.IaaSClient(), cloudClient.ProjectID(), cloudClient.Region(), stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(publicIPs).To(HaveLen(1)) securityGroups, err := cloudClient.ListSecurityGroupsByTags(ctx, stackitE2EBastionTags(testID)) @@ -556,7 +557,7 @@ var _ = Describe("Manager", Ordered, func() { loadBalancers, err := cloudClient.ListAPIServerLoadBalancersByTags(ctx, leakTags) g.Expect(err).NotTo(HaveOccurred()) g.Expect(loadBalancers).To(BeEmpty()) - publicIPs, err := cloudClient.ListPublicIPsByTags(ctx, stackitE2EBastionTags(testID)) + publicIPs, err := publicip.ListByTags(ctx, cloudClient.IaaSClient(), cloudClient.ProjectID(), cloudClient.Region(), stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(publicIPs).To(BeEmpty()) securityGroups, err := cloudClient.ListSecurityGroupsByTags(ctx, stackitE2EBastionTags(testID)) diff --git a/util/conditions.go b/util/conditions.go index f60b2bf..d22050e 100644 --- a/util/conditions.go +++ b/util/conditions.go @@ -14,11 +14,10 @@ import ( "errors" "time" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" - - "github.com/stackitcloud/cluster-api-provider-stackit/cloud" ) // SetCondition adds or updates a condition on conditions. From 235e1292fbdc51b975b29c8b71161bafcccd9ede Mon Sep 17 00:00:00 2001 From: Leonhard Schulze Date: Wed, 28 Oct 2026 13:54:43 +0100 Subject: [PATCH 2/6] fix: add legacy event api permssion to controller role --- config/rbac/role.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index b29226e..ba165ea 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -29,6 +29,13 @@ rules: verbs: - create - patch +- apiGroups: + - "" + resources: + - events + verbs: + - create + - patch - apiGroups: - infrastructure.cluster.x-k8s.io resources: From 36f9f8b6e44bb0d8074c68cc4ba30dc689330eb3 Mon Sep 17 00:00:00 2001 From: Leonhard Schulze Date: Wed, 28 Oct 2026 14:15:57 +0100 Subject: [PATCH 3/6] feat: create bastion host in a single availability zone and not metro --- cloud/fake/client.go | 15 ++++- cloud/sdk_client.go | 23 +++---- cloud/sdk_client_test.go | 20 +++--- cloud/services/bastion/bastion.go | 20 +++--- cloud/services/bastion/bastion_test.go | 68 ++++++++++++++++++++ cloud/types.go | 23 +++---- config/rbac/role.yaml | 20 ++---- controller/stackitcluster_controller.go | 1 + controller/stackitcluster_controller_test.go | 19 ++++++ controller/stackitmachine_controller.go | 1 + docs/src/topics/failure-domains.md | 5 ++ scope/cluster_scope.go | 23 +++++++ scope/machine_scope.go | 16 +---- 13 files changed, 183 insertions(+), 71 deletions(-) create mode 100644 cloud/services/bastion/bastion_test.go diff --git a/cloud/fake/client.go b/cloud/fake/client.go index 531ae31..0f5c589 100644 --- a/cloud/fake/client.go +++ b/cloud/fake/client.go @@ -75,6 +75,9 @@ type Client struct { // LastCreateServerInput records the most recent input passed to CreateServer. LastCreateServerInput cloud.CreateServerInput + // LastEnsureBastionInput records the most recent input passed to EnsureBastion. + LastEnsureBastionInput cloud.BastionInput + // Before* hooks, if non-nil, run before the call they belong to. They let a // test observe API server state at the exact moment a cloud call would // happen. Unlike FailNext*, they are not consumed. @@ -316,10 +319,16 @@ func (c *Client) EnsureBastion(_ context.Context, input cloud.BastionInput) (*cl } serverID := c.genID() + c.LastEnsureBastionInput = input + zone := input.AvailabilityZone + if zone == "" { + zone = "eu01-1" + } server := &cloud.Server{ - ID: serverID, - Name: input.Name, - State: "ACTIVE", + ID: serverID, + Name: input.Name, + State: "ACTIVE", + AvailabilityZone: zone, Addresses: []cloud.Address{ {Type: "InternalIP", Address: "10.0.0.20"}, }, diff --git a/cloud/sdk_client.go b/cloud/sdk_client.go index 0fa1751..531b7ec 100644 --- a/cloud/sdk_client.go +++ b/cloud/sdk_client.go @@ -268,17 +268,18 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas } server, err := c.CreateServer(ctx, CreateServerInput{ - Name: input.Name, - ProjectID: input.ProjectID, - Region: input.Region, - ImageID: input.ImageID, - MachineType: input.MachineType, - SSHKeyName: input.SSHKeyName, - NetworkID: input.NetworkID, - SecurityGroups: []string{securityGroup.ID}, - UserData: input.CloudInit, - Tags: input.Tags, - RootVolume: input.RootVolume, + Name: input.Name, + ProjectID: input.ProjectID, + Region: input.Region, + ImageID: input.ImageID, + MachineType: input.MachineType, + AvailabilityZone: input.AvailabilityZone, + SSHKeyName: input.SSHKeyName, + NetworkID: input.NetworkID, + SecurityGroups: []string{securityGroup.ID}, + UserData: input.CloudInit, + Tags: input.Tags, + RootVolume: input.RootVolume, }) if err != nil { return nil, err diff --git a/cloud/sdk_client_test.go b/cloud/sdk_client_test.go index ad0c9e7..3e235b7 100644 --- a/cloud/sdk_client_test.go +++ b/cloud/sdk_client_test.go @@ -512,15 +512,16 @@ func TestSDKClientEnsureBastionToleratesDuplicateSecurityGroupAttach(t *testing. client := newTestSDKClient(t, server.URL) bastion, err := client.EnsureBastion(context.Background(), BastionInput{ - Name: "bastion", - ProjectID: testSDKProjectID, - Region: testSDKRegion, - NetworkID: testSDKNetworkID, - ImageID: testSDKImageID, - MachineType: "c2i.1", - SSHKeyName: "default", - AllowedCIDRs: []string{"203.0.113.0/24"}, - Tags: map[string]string{"cluster": "test"}, + Name: "bastion", + ProjectID: testSDKProjectID, + Region: testSDKRegion, + NetworkID: testSDKNetworkID, + ImageID: testSDKImageID, + MachineType: "c2i.1", + AvailabilityZone: "eu01-1", + SSHKeyName: "default", + AllowedCIDRs: []string{"203.0.113.0/24"}, + Tags: map[string]string{"cluster": "test"}, }) if err != nil { t.Fatalf("EnsureBastion() error = %v", err) @@ -529,6 +530,7 @@ func TestSDKClientEnsureBastionToleratesDuplicateSecurityGroupAttach(t *testing. if attachCallCount != 1 { t.Fatalf("attach attempted %d time(s), want exactly 1 (the idempotent re-attach)", attachCallCount) } + assertStringField(t, createPayload, "availabilityZone", "eu01-1") groups, _ := createPayload["securityGroups"].([]any) if len(groups) != 1 || groups[0] != testSDKSecurityGroup { t.Fatalf("create payload securityGroups = %v, want [%s]", createPayload["securityGroups"], testSDKSecurityGroup) diff --git a/cloud/services/bastion/bastion.go b/cloud/services/bastion/bastion.go index 6b7f5d9..80eab63 100644 --- a/cloud/services/bastion/bastion.go +++ b/cloud/services/bastion/bastion.go @@ -13,6 +13,7 @@ package bastion import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + "github.com/stackitcloud/cluster-api-provider-stackit/scope" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -26,15 +27,16 @@ func Input(stackitCluster *infrav1.StackitCluster, cloudInit []byte) cloud.Basti tags[util.LabelResourceRole] = util.ResourceRoleBastion return cloud.BastionInput{ - Name: stackitCluster.Name + "-bastion", - ProjectID: stackitCluster.Spec.ProjectID, - Region: stackitCluster.Spec.Region, - NetworkID: stackitCluster.Spec.Network.ID, - ImageID: stackitCluster.Spec.Bastion.ImageID, - MachineType: stackitCluster.Spec.Bastion.MachineType, - SSHKeyName: stackitCluster.Spec.Bastion.SSHKeyName, - AllowedCIDRs: stackitCluster.Spec.Bastion.AllowedCIDRs, - Tags: tags, + Name: stackitCluster.Name + "-bastion", + ProjectID: stackitCluster.Spec.ProjectID, + Region: stackitCluster.Spec.Region, + NetworkID: stackitCluster.Spec.Network.ID, + ImageID: stackitCluster.Spec.Bastion.ImageID, + MachineType: stackitCluster.Spec.Bastion.MachineType, + AvailabilityZone: scope.DefaultAvailabilityZone(stackitCluster), + SSHKeyName: stackitCluster.Spec.Bastion.SSHKeyName, + AllowedCIDRs: stackitCluster.Spec.Bastion.AllowedCIDRs, + Tags: tags, RootVolume: cloud.RootVolumeInput{ SizeGiB: stackitCluster.Spec.Bastion.RootVolume.SizeGiB, PerformanceClass: stackitCluster.Spec.Bastion.RootVolume.PerformanceClass, diff --git a/cloud/services/bastion/bastion_test.go b/cloud/services/bastion/bastion_test.go new file mode 100644 index 0000000..88722a1 --- /dev/null +++ b/cloud/services/bastion/bastion_test.go @@ -0,0 +1,68 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package bastion + +import ( + "testing" + + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + + infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" +) + +func TestInputAvailabilityZone(t *testing.T) { + tests := []struct { + name string + region string + failureDomains []clusterv1.FailureDomain + wantAZ string + }{ + { + name: "selects first failure domain sorted alphabetically", + region: "eu01", + failureDomains: []clusterv1.FailureDomain{ + {Name: "eu01-3"}, + {Name: "eu01-2"}, + {Name: "eu01-1"}, + }, + wantAZ: "eu01-1", + }, + { + name: "falls back to region-1 when failureDomains is empty", + region: "eu02", + failureDomains: nil, + wantAZ: "eu02-1", + }, + { + name: "returns empty when region and failureDomains are empty", + region: "", + failureDomains: nil, + wantAZ: "", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cluster := &infrav1.StackitCluster{ + Spec: infrav1.StackitClusterSpec{ + Region: test.region, + }, + Status: infrav1.StackitClusterStatus{ + FailureDomains: test.failureDomains, + }, + } + input := Input(cluster, nil) + if input.AvailabilityZone != test.wantAZ { + t.Fatalf("Input().AvailabilityZone = %q, want %q", input.AvailabilityZone, test.wantAZ) + } + }) + } +} diff --git a/cloud/types.go b/cloud/types.go index 92a63dc..6cca29a 100644 --- a/cloud/types.go +++ b/cloud/types.go @@ -75,17 +75,18 @@ type CreateServerInput struct { // BastionInput holds all parameters required to ensure a bastion host. type BastionInput struct { - Name string - ProjectID string - Region string - NetworkID string - ImageID string - MachineType string - SSHKeyName string - AllowedCIDRs []string - Tags map[string]string - RootVolume RootVolumeInput - CloudInit []byte + Name string + ProjectID string + Region string + NetworkID string + ImageID string + MachineType string + AvailabilityZone string + SSHKeyName string + AllowedCIDRs []string + Tags map[string]string + RootVolume RootVolumeInput + CloudInit []byte } // NodeSSHAccessInput holds parameters required to allow SSH from the bastion diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index ba165ea..75a1891 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -14,15 +14,7 @@ rules: - list - watch - apiGroups: - - cluster.x-k8s.io - resources: - - clusters - - machines - verbs: - - get - - list - - watch -- apiGroups: + - "" - events.k8s.io resources: - events @@ -30,12 +22,14 @@ rules: - create - patch - apiGroups: - - "" + - cluster.x-k8s.io resources: - - events + - clusters + - machines verbs: - - create - - patch + - get + - list + - watch - apiGroups: - infrastructure.cluster.x-k8s.io resources: diff --git a/controller/stackitcluster_controller.go b/controller/stackitcluster_controller.go index a9e4a3c..8b35147 100644 --- a/controller/stackitcluster_controller.go +++ b/controller/stackitcluster_controller.go @@ -62,6 +62,7 @@ type StackitClusterReconciler struct { // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch // +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch func (r *StackitClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, err error) { log := logf.FromContext(ctx) diff --git a/controller/stackitcluster_controller_test.go b/controller/stackitcluster_controller_test.go index 65077b2..ccd6cf1 100644 --- a/controller/stackitcluster_controller_test.go +++ b/controller/stackitcluster_controller_test.go @@ -986,6 +986,25 @@ var _ = Describe("StackitCluster Controller", func() { })) }) + It("creates the bastion host in a single availability zone and not in metro (-m)", func() { + fakeCloud.AvailabilityZones = []string{"eu01-m", "eu01-3", "eu01-2"} + + got := &infrav1.StackitCluster{} + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + got.Spec.Bastion = validBastionSpec() + Expect(k8sClient.Update(ctx, got)).To(Succeed()) + + result, err := reconciler.Reconcile(ctx, request) + Expect(err).NotTo(HaveOccurred()) + Expect(result).To(Equal(reconcile.Result{})) + + Expect(fakeCloud.LastEnsureBastionInput.AvailabilityZone).To(Equal("eu01-2")) + Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) + server, err := fakeCloud.GetServer(ctx, got.Status.Bastion.ServerID) + Expect(err).NotTo(HaveOccurred()) + Expect(server.AvailabilityZone).To(Equal("eu01-2")) + }) + It("reports an error condition when listing availability zones fails", func() { fakeCloud.FailNextListAZ = fmt.Errorf("cloud failure: %w", cloud.ErrTransient) diff --git a/controller/stackitmachine_controller.go b/controller/stackitmachine_controller.go index 30688dd..cf71067 100644 --- a/controller/stackitmachine_controller.go +++ b/controller/stackitmachine_controller.go @@ -53,6 +53,7 @@ type StackitMachineReconciler struct { // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch // +kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch func (r *StackitMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, err error) { log := logf.FromContext(ctx) diff --git a/docs/src/topics/failure-domains.md b/docs/src/topics/failure-domains.md index 113e1fb..79e2ad3 100644 --- a/docs/src/topics/failure-domains.md +++ b/docs/src/topics/failure-domains.md @@ -51,6 +51,11 @@ using the following precedence: The actual zone where the VM was placed is surfaced on `StackitMachine.status.availabilityZone`. +When `StackitCluster.spec.bastion.enabled` is `true`, the provider-managed bastion +host is also placed into the cluster's default availability zone (the first +sorted published failure domain, e.g., `eu01-1`) so it is never created in a +metro zone (`-m`). + ## Worker Node Spreading Unlike the control plane, worker nodes do not automatically spread across failure domains diff --git a/scope/cluster_scope.go b/scope/cluster_scope.go index 12fc45a..0bb4c8b 100644 --- a/scope/cluster_scope.go +++ b/scope/cluster_scope.go @@ -15,6 +15,7 @@ package scope import ( "context" + "slices" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -102,6 +103,28 @@ func (s *ClusterScope) SetFailureDomains(failureDomains []clusterv1.FailureDomai s.StackitCluster.Status.FailureDomains = failureDomains } +// DefaultAvailabilityZone returns the default single availability zone for a +// StackitCluster (the alphabetically first published failure domain, falling +// back to -1). This prevents the STACKIT IaaS API from defaulting to +// the expensive metro zone (-m). +func DefaultAvailabilityZone(stackitCluster *infrav1.StackitCluster) string { + if stackitCluster == nil { + return "" + } + if len(stackitCluster.Status.FailureDomains) > 0 { + zones := make([]string, 0, len(stackitCluster.Status.FailureDomains)) + for _, fd := range stackitCluster.Status.FailureDomains { + zones = append(zones, fd.Name) + } + slices.Sort(zones) + return zones[0] + } + if stackitCluster.Spec.Region != "" { + return stackitCluster.Spec.Region + "-1" + } + return "" +} + func (s *ClusterScope) Cloud() cloud.Client { return s.CloudClient } diff --git a/scope/machine_scope.go b/scope/machine_scope.go index 58bafde..b736ce1 100644 --- a/scope/machine_scope.go +++ b/scope/machine_scope.go @@ -12,7 +12,6 @@ package scope import ( "context" - "slices" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -128,18 +127,5 @@ func (s *MachineScope) AvailabilityZone() string { return s.StackitMachine.Spec.AvailabilityZone } - if s.StackitCluster != nil && len(s.StackitCluster.Status.FailureDomains) > 0 { - zones := make([]string, 0, len(s.StackitCluster.Status.FailureDomains)) - for _, fd := range s.StackitCluster.Status.FailureDomains { - zones = append(zones, fd.Name) - } - slices.Sort(zones) - return zones[0] - } - - if s.StackitCluster != nil && s.StackitCluster.Spec.Region != "" { - return s.StackitCluster.Spec.Region + "-1" - } - - return "" + return DefaultAvailabilityZone(s.StackitCluster) } From 1f76439b48f30aaac969faa1447e75e6e0217beb Mon Sep 17 00:00:00 2001 From: Leonhard Schulze Date: Thu, 29 Oct 2026 11:33:01 +0100 Subject: [PATCH 4/6] docs: removed comments --- cloud/services/publicip/reconcile.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cloud/services/publicip/reconcile.go b/cloud/services/publicip/reconcile.go index d3f947c..449ee75 100644 --- a/cloud/services/publicip/reconcile.go +++ b/cloud/services/publicip/reconcile.go @@ -20,7 +20,6 @@ import ( "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" ) -// Reconcile reconciles cluster public IP resources. func (s *Service) Reconcile(ctx context.Context) error { log := logf.FromContext(ctx) if !s.scope.BastionSpec().Enabled { @@ -56,7 +55,6 @@ func (s *Service) Reconcile(ctx context.Context) error { return nil } -// Delete deletes cluster public IP resources. func (s *Service) Delete(ctx context.Context) error { log := logf.FromContext(ctx) log.V(1).Info("Deleting public IP resources") From 84012bf11493a9ae497eb4d186eaf5917bdbd562 Mon Sep 17 00:00:00 2001 From: Leonhard Schulze Date: Fri, 30 Oct 2026 10:02:50 +0100 Subject: [PATCH 5/6] feat: refactor security groups --- cloud/cleanup.go | 13 + cloud/cleanup_test.go | 20 + cloud/client.go | 6 - cloud/fake/client.go | 553 ++++++++++--- cloud/sdk_client.go | 383 +-------- cloud/sdk_client_test.go | 166 +--- cloud/services/bastion/bastion.go | 7 +- cloud/services/securitygroup/reconcile.go | 323 ++++++++ .../services/securitygroup/reconcile_test.go | 743 ++++++++++++++++++ cloud/services/securitygroup/rules.go | 198 +++++ cloud/services/securitygroup/service.go | 102 +++ cloud/types.go | 16 +- controller/stackitcluster_bastion.go | 24 +- controller/stackitcluster_controller_test.go | 66 +- controller/stackitcluster_infrastructure.go | 18 +- controller/stackitmachine_controller_test.go | 17 +- controller/stackitmachine_infrastructure.go | 14 +- scope/cluster_scope.go | 79 +- scope/machine_scope.go | 35 + test/e2e/e2e_test.go | 10 +- test/utils/utils.go | 9 + 21 files changed, 2106 insertions(+), 696 deletions(-) create mode 100644 cloud/services/securitygroup/reconcile.go create mode 100644 cloud/services/securitygroup/reconcile_test.go create mode 100644 cloud/services/securitygroup/rules.go create mode 100644 cloud/services/securitygroup/service.go diff --git a/cloud/cleanup.go b/cloud/cleanup.go index 3d635e0..481f3de 100644 --- a/cloud/cleanup.go +++ b/cloud/cleanup.go @@ -17,6 +17,7 @@ import ( "strings" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" ) // CleanupByTags deletes provider-managed cloud resources matching tags without @@ -76,6 +77,18 @@ func CleanupByTags(ctx context.Context, client Client, tags map[string]string) e } } + sgClient := securitygroup.NewClient(client) + securityGroups, err := sgClient.ListByTags(ctx, tags) + if err != nil { + errs = append(errs, err.Error()) + } else { + for _, sg := range securityGroups { + if err := sgClient.DeleteByID(ctx, sg.ID); err != nil { + errs = append(errs, fmt.Sprintf("delete security group %q: %v", sg.ID, err)) + } + } + } + if len(errs) > 0 { return fmt.Errorf("%w: %s", ErrTransient, strings.Join(errs, "; ")) } diff --git a/cloud/cleanup_test.go b/cloud/cleanup_test.go index 40f2304..2fe2dc0 100644 --- a/cloud/cleanup_test.go +++ b/cloud/cleanup_test.go @@ -42,6 +42,12 @@ func TestCleanupByTagsDeletesMatchingCloudResources(t *testing.T) { if _, err := client.CreatePublicIP(ctx, otherTags); err != nil { t.Fatalf("CreatePublicIP() error = %v", err) } + if _, err := client.CreateSecurityGroup(ctx, "matched-sg", tags); err != nil { + t.Fatalf("CreateSecurityGroup() error = %v", err) + } + if _, err := client.CreateSecurityGroup(ctx, "other-sg", otherTags); err != nil { + t.Fatalf("CreateSecurityGroup() error = %v", err) + } if _, err := client.EnsureAPIServerLoadBalancer(ctx, cloud.LoadBalancerInput{ Name: "matched", Tags: tags, @@ -106,4 +112,18 @@ func TestCleanupByTagsDeletesMatchingCloudResources(t *testing.T) { if len(otherPublicIPs) != 1 { t.Fatalf("other public IP count = %d, want 1", len(otherPublicIPs)) } + matchedSecurityGroups, err := client.ListSecurityGroupsByTags(ctx, tags) + if err != nil { + t.Fatalf("ListSecurityGroupsByTags() error = %v", err) + } + if len(matchedSecurityGroups) != 0 { + t.Fatalf("matched security group count = %d, want 0", len(matchedSecurityGroups)) + } + otherSecurityGroups, err := client.ListSecurityGroupsByTags(ctx, otherTags) + if err != nil { + t.Fatalf("ListSecurityGroupsByTags(other) error = %v", err) + } + if len(otherSecurityGroups) != 1 { + t.Fatalf("other security group count = %d, want 1", len(otherSecurityGroups)) + } } diff --git a/cloud/client.go b/cloud/client.go index 7e5bd73..40b657a 100644 --- a/cloud/client.go +++ b/cloud/client.go @@ -55,12 +55,6 @@ type Client interface { DeleteBastion(ctx context.Context, input BastionInput, status Bastion) error - EnsureNodeSSHAccess(ctx context.Context, input NodeSSHAccessInput) (*SecurityGroup, error) - - DeleteNodeSSHAccess(ctx context.Context, tags map[string]string) error - - ListSecurityGroupsByTags(ctx context.Context, tags map[string]string) ([]*SecurityGroup, error) - EnsureAPIServerLoadBalancer(ctx context.Context, input LoadBalancerInput) (*LoadBalancer, error) ListAPIServerLoadBalancersByTags(ctx context.Context, tags map[string]string) ([]*LoadBalancer, error) diff --git a/cloud/fake/client.go b/cloud/fake/client.go index 0f5c589..5671191 100644 --- a/cloud/fake/client.go +++ b/cloud/fake/client.go @@ -24,6 +24,7 @@ import ( "sync" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/stackit-sdk-go/core/config" iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) @@ -48,25 +49,24 @@ type Client struct { // failure injection: if non-nil, the next call returns this error and the // field is cleared. - FailNextCreateServer error - FailNextDeleteServer error - FailNextGetServer error - FailNextFindServer error - FailNextEnsureLB error - FailNextDeleteLB error - FailNextSetTargets error - FailNextGetNetwork error - FailNextEnsureBastion error - FailNextDeleteBastion error - FailNextEnsureNodeSSH error - FailNextDeleteNodeSSH error - FailNextListAZ error - FailNextGetPublicIP error - FailNextListPublicIPs error - FailNextCreatePublicIP error - FailNextDeletePublicIP error - FailNextAddPublicIP error - FailNextRemovePublicIP error + FailNextCreateServer error + FailNextDeleteServer error + FailNextGetServer error + FailNextFindServer error + FailNextEnsureLB error + FailNextDeleteLB error + FailNextSetTargets error + FailNextGetNetwork error + FailNextEnsureBastion error + FailNextDeleteBastion error + FailNextAddSecurityGroup error + FailNextListAZ error + FailNextGetPublicIP error + FailNextListPublicIPs error + FailNextCreatePublicIP error + FailNextDeletePublicIP error + FailNextAddPublicIP error + FailNextRemovePublicIP error // AvailabilityZones is returned by ListAvailabilityZones if non-empty. // Defaults to eu01-1, eu01-2, eu01-3. @@ -86,9 +86,9 @@ type Client struct { // CreateServerCalls counts successful CreateServer calls (for idempotency // assertions). - CreateServerCalls int - EnsureBastionCalls int - EnsureNodeSSHCalls int + CreateServerCalls int + EnsureBastionCalls int + AddSecurityGroupCalls int } type serverEntry struct { @@ -109,11 +109,18 @@ type publicIPEntry struct { tags map[string]string } +type securityGroupRuleEntry struct { + id string + securitygroup.Rule +} + type securityGroupEntry struct { securityGroup *cloud.SecurityGroup + description string tags map[string]string allowedCIDRs []string remoteSecurityGroups []string + rules map[string]*securityGroupRuleEntry } // New returns a Client preconfigured with the given networks. @@ -295,29 +302,27 @@ func (c *Client) EnsureBastion(_ context.Context, input cloud.BastionInput) (*cl if err := consume(&c.FailNextEnsureBastion); err != nil { return nil, err } + securityGroupID := input.SecurityGroupID + if securityGroupID == "" { + securityGroupID = idOfSecurityGroup(c.securityGroupByTags(input.Tags)) + } for _, serverEntry := range c.servers { if len(input.Tags) > 0 && mapContains(serverEntry.tags, input.Tags) { - securityGroup := c.securityGroupByTags(input.Tags) + if securityGroupID != "" { + if serverEntry.securityGroups == nil { + serverEntry.securityGroups = map[string]struct{}{} + } + serverEntry.securityGroups[securityGroupID] = struct{}{} + } return &cloud.Bastion{ ServerID: serverEntry.server.ID, ServerState: serverEntry.server.State, ServerPowerStatus: serverEntry.server.PowerStatus, - SecurityGroupID: idOfSecurityGroup(securityGroup), + SecurityGroupID: securityGroupID, }, nil } } - securityGroupID := c.genID() - securityGroup := &cloud.SecurityGroup{ - ID: securityGroupID, - Name: input.Name + "-ssh", - } - c.securityGroups[securityGroupID] = &securityGroupEntry{ - securityGroup: securityGroup, - tags: copyTags(input.Tags), - allowedCIDRs: append([]string(nil), input.AllowedCIDRs...), - } - serverID := c.genID() c.LastEnsureBastionInput = input zone := input.AvailabilityZone @@ -333,10 +338,14 @@ func (c *Client) EnsureBastion(_ context.Context, input cloud.BastionInput) (*cl {Type: "InternalIP", Address: "10.0.0.20"}, }, } + var sgSet map[string]struct{} + if securityGroupID != "" { + sgSet = securityGroupSet([]string{securityGroupID}) + } c.servers[serverID] = &serverEntry{ server: server, tags: copyTags(input.Tags), - securityGroups: securityGroupSet([]string{securityGroupID}), + securityGroups: sgSet, userData: append([]byte(nil), input.CloudInit...), } @@ -358,74 +367,11 @@ func (c *Client) DeleteBastion(_ context.Context, input cloud.BastionInput, stat if status.ServerID != "" { delete(c.servers, status.ServerID) } - if status.SecurityGroupID != "" { - delete(c.securityGroups, status.SecurityGroupID) - } for id, entry := range c.servers { - if mapContains(entry.tags, input.Tags) { + if len(input.Tags) > 0 && mapContains(entry.tags, input.Tags) { delete(c.servers, id) } } - for id, entry := range c.securityGroups { - if mapContains(entry.tags, input.Tags) { - delete(c.securityGroups, id) - } - } - return nil -} - -func (c *Client) EnsureNodeSSHAccess(_ context.Context, input cloud.NodeSSHAccessInput) (*cloud.SecurityGroup, error) { - c.mu.Lock() - defer c.mu.Unlock() - - if err := consume(&c.FailNextEnsureNodeSSH); err != nil { - return nil, err - } - serverEntry, ok := c.servers[input.ServerID] - if !ok { - return nil, fmt.Errorf("server %q: %w", input.ServerID, cloud.ErrNotFound) - } - securityGroup := c.securityGroupByTags(input.Tags) - if securityGroup == nil { - securityGroup = &cloud.SecurityGroup{ - ID: c.genID(), - Name: input.Name, - } - c.securityGroups[securityGroup.ID] = &securityGroupEntry{ - securityGroup: securityGroup, - tags: copyTags(input.Tags), - remoteSecurityGroups: []string{input.BastionSecurityGroupID}, - } - } else { - entry := c.securityGroups[securityGroup.ID] - if !containsString(entry.remoteSecurityGroups, input.BastionSecurityGroupID) { - entry.remoteSecurityGroups = append(entry.remoteSecurityGroups, input.BastionSecurityGroupID) - } - } - if serverEntry.securityGroups == nil { - serverEntry.securityGroups = map[string]struct{}{} - } - serverEntry.securityGroups[securityGroup.ID] = struct{}{} - c.EnsureNodeSSHCalls++ - out := *securityGroup - return &out, nil -} - -func (c *Client) DeleteNodeSSHAccess(_ context.Context, tags map[string]string) error { - c.mu.Lock() - defer c.mu.Unlock() - - if err := consume(&c.FailNextDeleteNodeSSH); err != nil { - return err - } - for id, entry := range c.securityGroups { - if mapContains(entry.tags, tags) { - delete(c.securityGroups, id) - for _, server := range c.servers { - delete(server.securityGroups, id) - } - } - } return nil } @@ -560,6 +506,18 @@ func (c *Client) ListSecurityGroupsByTags(_ context.Context, tags map[string]str return securityGroups, nil } +func (c *Client) CreateSecurityGroup(_ context.Context, name string, tags map[string]string) (*cloud.SecurityGroup, error) { + c.mu.Lock() + defer c.mu.Unlock() + + entry, err := c.createSecurityGroupEntry(name, "", tags) + if err != nil { + return nil, err + } + out := *entry.securityGroup + return &out, nil +} + func (c *Client) EnsureAPIServerLoadBalancer( _ context.Context, input cloud.LoadBalancerInput, @@ -736,6 +694,21 @@ func (c *Client) SecurityGroupRemoteSources(securityGroupID string) []string { return append([]string(nil), entry.remoteSecurityGroups...) } +// SecurityGroupRules returns the rules currently stored on a fake security group. +func (c *Client) SecurityGroupRules(securityGroupID string) []securitygroup.Rule { + c.mu.Lock() + defer c.mu.Unlock() + entry, ok := c.securityGroups[securityGroupID] + if !ok { + return nil + } + rules := make([]securitygroup.Rule, 0, len(entry.rules)) + for _, r := range entry.rules { + rules = append(rules, r.Rule) + } + return rules +} + // LoadBalancerTargetCount returns the number of targets in one load balancer // target pool (test helper). func (c *Client) LoadBalancerTargetCount(id string) int { @@ -850,6 +823,17 @@ func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { rec := httptest.NewRecorder() rec.Header().Set("Content-Type", "application/json") + + if c.handlePublicIPRequest(rec, r) || c.handleSecurityGroupRequest(rec, r) { + return rec.Result(), nil + } + + rec.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(rec).Encode(map[string]any{"message": "not found"}) + return rec.Result(), nil +} + +func (c *Client) handlePublicIPRequest(rec *httptest.ResponseRecorder, r *http.Request) bool { path := r.URL.Path switch { @@ -866,11 +850,8 @@ func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { default: err = cloud.ErrInvalidInput } - if err != nil { - writeFakeError(rec, err) - } else { - rec.WriteHeader(http.StatusNoContent) - } + writeFakeStatus(rec, err) + return true case r.Method == http.MethodPost && strings.HasSuffix(path, "/public-ips"): var body struct { @@ -885,6 +866,7 @@ func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { } else { _ = json.NewEncoder(rec).Encode(fakePublicIPMap(created, body.Labels)) } + return true case r.Method == http.MethodGet && strings.HasSuffix(path, "/public-ips"): tags := parseLabelSelector(r.URL.Query().Get("label_selector")) @@ -894,6 +876,7 @@ func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { } else { _ = json.NewEncoder(rec).Encode(map[string]any{"items": items}) } + return true case r.Method == http.MethodGet && strings.Contains(path, "/public-ips/"): id := path[strings.LastIndex(path, "/public-ips/")+len("/public-ips/"):] @@ -903,21 +886,151 @@ func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { } else { _ = json.NewEncoder(rec).Encode(fakePublicIPMap(got, nil)) } + return true case r.Method == http.MethodDelete && strings.Contains(path, "/public-ips/"): id := path[strings.LastIndex(path, "/public-ips/")+len("/public-ips/"):] - if err := c.DeletePublicIP(r.Context(), id); err != nil { + writeFakeStatus(rec, c.DeletePublicIP(r.Context(), id)) + return true + + default: + return false + } +} + +func (c *Client) handleSecurityGroupRequest(rec *httptest.ResponseRecorder, r *http.Request) bool { + path := r.URL.Path + + switch { + case strings.Contains(path, "/servers/") && strings.Contains(path, "/security-groups/"): + parts := strings.Split(path, "/servers/") + sub := strings.Split(parts[1], "/security-groups/") + serverID, securityGroupID := sub[0], sub[1] + var err error + switch r.Method { + case http.MethodPut: + err = c.addSecurityGroupToServer(serverID, securityGroupID) + case http.MethodDelete: + err = c.removeSecurityGroupFromServer(serverID, securityGroupID) + default: + err = cloud.ErrInvalidInput + } + writeFakeStatus(rec, err) + return true + + case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): + tags := parseLabelSelector(r.URL.Query().Get("label_selector")) + items, err := c.listServerItems(tags) + if err != nil { writeFakeError(rec, err) } else { - rec.WriteHeader(http.StatusNoContent) + _ = json.NewEncoder(rec).Encode(map[string]any{"items": items}) } + return true + + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups"): + var body struct { + Name string `json:"name"` + Description string `json:"description"` + Labels map[string]string `json:"labels"` + } + if r.Body != nil { + _ = json.NewDecoder(r.Body).Decode(&body) + } + created, err := c.createSecurityGroup(body.Name, body.Description, body.Labels) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(created) + } + return true + + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + tags := parseLabelSelector(r.URL.Query().Get("label_selector")) + items, err := c.listSecurityGroupItems(tags) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(map[string]any{"items": items}) + } + return true + + case strings.Contains(path, "/security-groups/") && strings.HasSuffix(path, "/rules") && r.Method == http.MethodGet: + sub := path[strings.Index(path, "/security-groups/")+len("/security-groups/"):] + sgID := strings.TrimSuffix(sub, "/rules") + items, err := c.listSecurityGroupRuleItems(sgID) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(map[string]any{"items": items}) + } + return true + + case strings.Contains(path, "/security-groups/") && strings.HasSuffix(path, "/rules") && r.Method == http.MethodPost: + c.handleCreateSecurityGroupRule(rec, r) + return true + + case strings.Contains(path, "/security-groups/") && strings.Contains(path, "/rules/") && r.Method == http.MethodDelete: + sub := path[strings.Index(path, "/security-groups/")+len("/security-groups/"):] + parts := strings.SplitN(sub, "/rules/", 2) + writeFakeStatus(rec, c.deleteSecurityGroupRule(parts[0], parts[1])) + return true + + case r.Method == http.MethodDelete && strings.Contains(path, "/security-groups/"): + id := path[strings.LastIndex(path, "/security-groups/")+len("/security-groups/"):] + writeFakeStatus(rec, c.deleteSecurityGroup(id)) + return true default: - rec.WriteHeader(http.StatusNotFound) - _ = json.NewEncoder(rec).Encode(map[string]any{"message": "not found"}) + return false } +} - return rec.Result(), nil +func (c *Client) handleCreateSecurityGroupRule(rec *httptest.ResponseRecorder, r *http.Request) { + path := r.URL.Path + sub := path[strings.Index(path, "/security-groups/")+len("/security-groups/"):] + sgID := strings.TrimSuffix(sub, "/rules") + var body struct { + Direction string `json:"direction"` + Protocol string `json:"protocol"` + PortRange *struct { + Min int64 `json:"min"` + Max int64 `json:"max"` + } `json:"portRange"` + IPRange string `json:"ipRange"` + RemoteSecurityGroupID string `json:"remoteSecurityGroupId"` + } + if r.Body != nil { + _ = json.NewDecoder(r.Body).Decode(&body) + } + var portMin, portMax int64 + if body.PortRange != nil { + portMin = body.PortRange.Min + portMax = body.PortRange.Max + } + created, err := c.createSecurityGroupRule(sgID, securityGroupRuleEntry{ + Rule: securitygroup.Rule{ + Direction: securitygroup.Direction(body.Direction), + Protocol: securitygroup.Protocol(body.Protocol), + PortMin: portMin, + PortMax: portMax, + IPRange: body.IPRange, + RemoteSecurityGroupID: body.RemoteSecurityGroupID, + }, + }) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(created) + } +} + +func writeFakeStatus(rec *httptest.ResponseRecorder, err error) { + if err != nil { + writeFakeError(rec, err) + } else { + rec.WriteHeader(http.StatusNoContent) + } } func (c *Client) listPublicIPItems(tags map[string]string) ([]map[string]any, error) { @@ -957,6 +1070,226 @@ func fakePublicIPMap(p *cloud.PublicIP, tags map[string]string) map[string]any { return m } +func (c *Client) addSecurityGroupToServer(serverID, securityGroupID string) error { + c.mu.Lock() + defer c.mu.Unlock() + + if err := consume(&c.FailNextAddSecurityGroup); err != nil { + return err + } + serverEntry, ok := c.servers[serverID] + if !ok { + return fmt.Errorf("server %q: %w", serverID, cloud.ErrNotFound) + } + if _, ok := c.securityGroups[securityGroupID]; !ok { + return fmt.Errorf("security group %q: %w", securityGroupID, cloud.ErrNotFound) + } + if serverEntry.securityGroups == nil { + serverEntry.securityGroups = map[string]struct{}{} + } + serverEntry.securityGroups[securityGroupID] = struct{}{} + c.AddSecurityGroupCalls++ + return nil +} + +func (c *Client) listServerItems(tags map[string]string) ([]map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + items := make([]map[string]any, 0, len(c.servers)) + for _, entry := range c.servers { + if len(tags) > 0 && !mapContains(entry.tags, tags) { + continue + } + m := map[string]any{ + "id": entry.server.ID, + "name": entry.server.Name, + "machineType": "c2i.1", + "status": entry.server.State, + } + if len(entry.tags) > 0 { + labels := make(map[string]any, len(entry.tags)) + for k, v := range entry.tags { + labels[k] = v + } + m["labels"] = labels + } + items = append(items, m) + } + return items, nil +} + +func (c *Client) removeSecurityGroupFromServer(serverID, securityGroupID string) error { + c.mu.Lock() + defer c.mu.Unlock() + + serverEntry, ok := c.servers[serverID] + if !ok { + return fmt.Errorf("server %q: %w", serverID, cloud.ErrNotFound) + } + delete(serverEntry.securityGroups, securityGroupID) + return nil +} + +func (c *Client) createSecurityGroup(name, description string, tags map[string]string) (map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + entry, err := c.createSecurityGroupEntry(name, description, tags) + if err != nil { + return nil, err + } + return fakeSecurityGroupMap(entry), nil +} + +func (c *Client) createSecurityGroupEntry(name, description string, tags map[string]string) (*securityGroupEntry, error) { + if name == "" { + return nil, fmt.Errorf("security group name is empty: %w", cloud.ErrInvalidInput) + } + id := c.genID() + sg := &cloud.SecurityGroup{ + ID: id, + Name: name, + } + entry := &securityGroupEntry{ + securityGroup: sg, + description: description, + tags: copyTags(tags), + rules: make(map[string]*securityGroupRuleEntry), + } + c.securityGroups[id] = entry + return entry, nil +} + +func (c *Client) listSecurityGroupItems(tags map[string]string) ([]map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if len(tags) == 0 { + return nil, fmt.Errorf("empty tags: %w", cloud.ErrInvalidInput) + } + items := make([]map[string]any, 0, len(c.securityGroups)) + for _, entry := range c.securityGroups { + if mapContains(entry.tags, tags) { + items = append(items, fakeSecurityGroupMap(entry)) + } + } + return items, nil +} + +func (c *Client) listSecurityGroupRuleItems(sgID string) ([]map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + entry, ok := c.securityGroups[sgID] + if !ok { + return nil, fmt.Errorf("security group %q: %w", sgID, cloud.ErrNotFound) + } + items := make([]map[string]any, 0, len(entry.rules)) + for _, rule := range entry.rules { + items = append(items, fakeSecurityGroupRuleMap(rule)) + } + return items, nil +} + +func (c *Client) createSecurityGroupRule(sgID string, rule securityGroupRuleEntry) (map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + entry, ok := c.securityGroups[sgID] + if !ok { + return nil, fmt.Errorf("security group %q: %w", sgID, cloud.ErrNotFound) + } + if entry.rules == nil { + entry.rules = make(map[string]*securityGroupRuleEntry) + } + rule.id = c.genID() + stored := rule + entry.rules[rule.id] = &stored + if rule.IPRange != "" && !containsString(entry.allowedCIDRs, rule.IPRange) { + entry.allowedCIDRs = append(entry.allowedCIDRs, rule.IPRange) + } + if rule.RemoteSecurityGroupID != "" && !containsString(entry.remoteSecurityGroups, rule.RemoteSecurityGroupID) { + entry.remoteSecurityGroups = append(entry.remoteSecurityGroups, rule.RemoteSecurityGroupID) + } + return fakeSecurityGroupRuleMap(&stored), nil +} + +func (c *Client) deleteSecurityGroupRule(sgID, ruleID string) error { + c.mu.Lock() + defer c.mu.Unlock() + + entry, ok := c.securityGroups[sgID] + if !ok { + return fmt.Errorf("security group %q: %w", sgID, cloud.ErrNotFound) + } + if entry.rules == nil { + return fmt.Errorf("rule %q: %w", ruleID, cloud.ErrNotFound) + } + if _, ok := entry.rules[ruleID]; !ok { + return fmt.Errorf("rule %q: %w", ruleID, cloud.ErrNotFound) + } + delete(entry.rules, ruleID) + return nil +} + +func (c *Client) deleteSecurityGroup(id string) error { + c.mu.Lock() + defer c.mu.Unlock() + + if id == "" { + return fmt.Errorf("security group ID is empty: %w", cloud.ErrInvalidInput) + } + if _, ok := c.securityGroups[id]; !ok { + return fmt.Errorf("security group %q: %w", id, cloud.ErrNotFound) + } + for serverID, server := range c.servers { + if _, attached := server.securityGroups[id]; attached { + return fmt.Errorf("security group %q is still attached to server %q: %w", id, serverID, cloud.ErrConflict) + } + } + delete(c.securityGroups, id) + return nil +} + +func fakeSecurityGroupMap(entry *securityGroupEntry) map[string]any { + m := map[string]any{ + "id": entry.securityGroup.ID, + "name": entry.securityGroup.Name, + } + if entry.description != "" { + m["description"] = entry.description + } + if len(entry.tags) > 0 { + labels := make(map[string]any, len(entry.tags)) + for k, v := range entry.tags { + labels[k] = v + } + m["labels"] = labels + } + return m +} + +func fakeSecurityGroupRuleMap(rule *securityGroupRuleEntry) map[string]any { + m := map[string]any{ + "id": rule.id, + "direction": string(rule.Direction), + } + if rule.Protocol != "" { + m["protocol"] = map[string]any{"name": string(rule.Protocol)} + } + if rule.PortMin > 0 || rule.PortMax > 0 { + m["portRange"] = map[string]any{"min": rule.PortMin, "max": rule.PortMax} + } + if rule.IPRange != "" { + m["ipRange"] = rule.IPRange + } + if rule.RemoteSecurityGroupID != "" { + m["remoteSecurityGroupId"] = rule.RemoteSecurityGroupID + } + return m +} + func parseLabelSelector(selector string) map[string]string { if selector == "" { return nil diff --git a/cloud/sdk_client.go b/cloud/sdk_client.go index 531b7ec..4777106 100644 --- a/cloud/sdk_client.go +++ b/cloud/sdk_client.go @@ -19,6 +19,7 @@ import ( "slices" "strings" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" "github.com/stackitcloud/stackit-sdk-go/core/config" iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" @@ -35,8 +36,6 @@ const ( apiserverListenerName = "apiserver" bootstrapTargetName = "capi-bootstrap-placeholder" bootstrapTargetIP = "10.0.0.1" - sshPort = 22 - tcpProtocol = "tcp" ) // SDKClient is the STACKIT SDK-backed implementation of Client. @@ -259,12 +258,14 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas return nil, fmt.Errorf("%w: bastion tags are required", ErrInvalidInput) } - securityGroup, err := c.ensureBastionSecurityGroup(ctx, input) - if err != nil { - return nil, err - } - if err := c.ensureBastionSecurityGroupRules(ctx, securityGroup.ID, input.AllowedCIDRs); err != nil { - return nil, err + sgClient := securitygroup.NewClient(c) + securityGroupID := input.SecurityGroupID + if securityGroupID == "" { + securityGroup, err := sgClient.FindByTags(ctx, input.Tags) + if err != nil { + return nil, err + } + securityGroupID = securityGroup.ID } server, err := c.CreateServer(ctx, CreateServerInput{ @@ -276,7 +277,7 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas AvailabilityZone: input.AvailabilityZone, SSHKeyName: input.SSHKeyName, NetworkID: input.NetworkID, - SecurityGroups: []string{securityGroup.ID}, + SecurityGroups: []string{securityGroupID}, UserData: input.CloudInit, Tags: input.Tags, RootVolume: input.RootVolume, @@ -290,7 +291,7 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas // by an interrupted DeleteBastion). Kept deliberately, and idempotent: the // duplicate/no-port-yet responses are tolerated so they can no longer abort // the reconcile before the public IP below is assigned. - if err := c.addSecurityGroupToServer(ctx, server.ID, securityGroup.ID); err != nil { + if err := sgClient.AttachToServer(ctx, server.ID, securityGroupID); err != nil { return nil, err } @@ -298,7 +299,7 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas ServerID: server.ID, ServerState: server.State, ServerPowerStatus: server.PowerStatus, - SecurityGroupID: securityGroup.ID, + SecurityGroupID: securityGroupID, }, nil } @@ -312,10 +313,17 @@ func (c *SDKClient) DeleteBastion(ctx context.Context, input BastionInput, statu return err } } + if serverID == "" { + return nil + } + sgClient := securitygroup.NewClient(c) securityGroupID := status.SecurityGroupID if securityGroupID == "" { - securityGroup, err := c.findSecurityGroupByTags(ctx, input.Tags) + securityGroupID = input.SecurityGroupID + } + if securityGroupID == "" && len(input.Tags) > 0 { + securityGroup, err := sgClient.FindByTags(ctx, input.Tags) if err == nil { securityGroupID = securityGroup.ID } else if !IsNotFound(err) { @@ -323,117 +331,17 @@ func (c *SDKClient) DeleteBastion(ctx context.Context, input BastionInput, statu } } - if serverID != "" && securityGroupID != "" { - if err := c.iaasClient.DefaultAPI. - RemoveSecurityGroupFromServer(ctx, c.projectID, c.region, serverID, securityGroupID). - Execute(); err != nil { - err := shared.ClassifySDKError("remove security group from server", err) - if !IsNotFound(err) && !IsConflict(err) { - return err - } - } - } - if serverID != "" { - if err := c.DeleteServer(ctx, serverID); err != nil && !IsNotFound(err) { - return err - } - } if securityGroupID != "" { - if err := c.deleteSecurityGroupRules(ctx, securityGroupID); err != nil { + if err := sgClient.DetachFromServer(ctx, serverID, securityGroupID); err != nil { return err } - if err := c.iaasClient.DefaultAPI. - DeleteSecurityGroup(ctx, c.projectID, c.region, securityGroupID). - Execute(); err != nil { - err := shared.ClassifySDKError("delete security group", err) - if !IsNotFound(err) { - return err - } - } - } - return nil -} - -func (c *SDKClient) EnsureNodeSSHAccess(ctx context.Context, input NodeSSHAccessInput) (*SecurityGroup, error) { - if input.Name == "" || input.ServerID == "" || input.BastionSecurityGroupID == "" { - return nil, fmt.Errorf( - "%w: node SSH access name, server ID, and bastion security group ID are required", - ErrInvalidInput, - ) - } - if len(input.Tags) == 0 { - return nil, fmt.Errorf("%w: node SSH access tags are required", ErrInvalidInput) - } - - securityGroup, err := c.ensureNodeSSHSecurityGroup(ctx, input) - if err != nil { - return nil, err - } - if err := c.ensureNodeSSHSecurityGroupRule(ctx, securityGroup.ID, input.BastionSecurityGroupID); err != nil { - return nil, err - } - if err := c.addSecurityGroupToServer(ctx, input.ServerID, securityGroup.ID); err != nil { - return nil, err - } - return securityGroup, nil -} - -func (c *SDKClient) DeleteNodeSSHAccess(ctx context.Context, tags map[string]string) error { - securityGroup, err := c.findSecurityGroupByTags(ctx, tags) - if IsNotFound(err) { - return nil - } - if err != nil { - return err - } - servers, err := c.ListServersByTags(ctx, clusterServerSelectorFromTags(tags)) - if err != nil { - return err - } - for _, server := range servers { - if err := c.iaasClient.DefaultAPI. - RemoveSecurityGroupFromServer(ctx, c.projectID, c.region, server.ID, securityGroup.ID). - Execute(); err != nil { - err := shared.ClassifySDKError("remove security group from server", err) - if !IsNotFound(err) && !IsConflict(err) { - return err - } - } } - if err := c.deleteSecurityGroupRules(ctx, securityGroup.ID); err != nil { + if err := c.DeleteServer(ctx, serverID); err != nil && !IsNotFound(err) { return err } - if err := c.iaasClient.DefaultAPI. - DeleteSecurityGroup(ctx, c.projectID, c.region, securityGroup.ID). - Execute(); err != nil { - err := shared.ClassifySDKError("delete security group", err) - if !IsNotFound(err) { - return err - } - } return nil } -func (c *SDKClient) ListSecurityGroupsByTags(ctx context.Context, tags map[string]string) ([]*SecurityGroup, error) { - if len(tags) == 0 { - return nil, fmt.Errorf("%w: empty tag selector", ErrInvalidInput) - } - resp, err := c.iaasClient.DefaultAPI.ListSecurityGroups(ctx, c.projectID, c.region). - LabelSelector(shared.LabelSelector(tags)). - Execute() - if err != nil { - return nil, shared.ClassifySDKError("list security groups", err) - } - matched := []*SecurityGroup{} - for _, securityGroup := range resp.GetItems() { - if shared.LabelsContainTags(securityGroup.GetLabels(), tags) { - securityGroup := securityGroup - matched = append(matched, securityGroupFromSDK(&securityGroup)) - } - } - return matched, nil -} - func (c *SDKClient) EnsureAPIServerLoadBalancer(ctx context.Context, input LoadBalancerInput) (*LoadBalancer, error) { if existing, err := c.findLoadBalancerByTags(ctx, input.Tags); err == nil { return existing, nil @@ -604,229 +512,6 @@ func (c *SDKClient) findLoadBalancerByTags(ctx context.Context, tags map[string] return matched[0], nil } -func (c *SDKClient) ensureBastionSecurityGroup(ctx context.Context, input BastionInput) (*SecurityGroup, error) { - if existing, err := c.findSecurityGroupByTags(ctx, input.Tags); err == nil { - return existing, nil - } else if !IsNotFound(err) { - return nil, err - } - payload := iaas.NewCreateSecurityGroupPayload(input.Name + "-ssh") - payload.SetDescription("Cluster API Provider STACKIT bastion SSH access") - payload.SetLabels(shared.TagsToSDKLabels(input.Tags)) - securityGroup, err := c.iaasClient.DefaultAPI.CreateSecurityGroup(ctx, c.projectID, c.region). - CreateSecurityGroupPayload(*payload). - Execute() - if err != nil { - return nil, shared.ClassifySDKError("create security group", err) - } - return securityGroupFromSDK(securityGroup), nil -} - -func (c *SDKClient) ensureNodeSSHSecurityGroup(ctx context.Context, input NodeSSHAccessInput) (*SecurityGroup, error) { - if existing, err := c.findSecurityGroupByTags(ctx, input.Tags); err == nil { - return existing, nil - } else if !IsNotFound(err) { - return nil, err - } - payload := iaas.NewCreateSecurityGroupPayload(input.Name) - payload.SetDescription("Cluster API Provider STACKIT node SSH access from bastion") - payload.SetLabels(shared.TagsToSDKLabels(input.Tags)) - securityGroup, err := c.iaasClient.DefaultAPI.CreateSecurityGroup(ctx, c.projectID, c.region). - CreateSecurityGroupPayload(*payload). - Execute() - if err != nil { - return nil, shared.ClassifySDKError("create security group", err) - } - return securityGroupFromSDK(securityGroup), nil -} - -func (c *SDKClient) ensureBastionSecurityGroupRules(ctx context.Context, securityGroupID string, cidrs []string) error { - if securityGroupID == "" { - return fmt.Errorf("%w: security group ID is required", ErrInvalidInput) - } - resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, securityGroupID).Execute() - if err != nil { - return shared.ClassifySDKError("list security group rules", err) - } - existingRules := resp.GetItems() - desired := make(map[string]struct{}, len(cidrs)) - for _, cidr := range cidrs { - if cidr == "" { - return fmt.Errorf("%w: empty bastion allowed CIDR", ErrInvalidInput) - } - if _, seen := desired[cidr]; seen { - // A CIDR listed twice would otherwise be created twice: - // existingRules is a snapshot from before this loop, so the second - // pass does not see the rule the first pass just created, and the - // duplicate create fails the whole bastion reconcile. - continue - } - desired[cidr] = struct{}{} - if hasSSHRule(existingRules, cidr) { - continue - } - protocol := iaas.StringAsCreateProtocol(new(tcpProtocol)) - rule := iaas.NewCreateSecurityGroupRulePayload("ingress") - rule.SetIpRange(cidr) - rule.SetPortRange(*iaas.NewPortRange(sshPort, sshPort)) - rule.SetProtocol(protocol) - if _, err := c.iaasClient.DefaultAPI.CreateSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID). - CreateSecurityGroupRulePayload(*rule). - Execute(); err != nil { - return shared.ClassifySDKError("create security group rule", err) - } - } - - // Revoke SSH rules for CIDRs that are no longer desired. Without this the - // rule set only ever grows, so narrowing allowedCIDRs would not actually - // take access away from the previously allowed range. - for _, rule := range existingRules { - if !isSSHRule(rule) { - continue - } - if _, keep := desired[rule.GetIpRange()]; keep { - continue - } - ruleID := rule.GetId() - if ruleID == "" { - continue - } - if err := c.iaasClient.DefaultAPI. - DeleteSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID, ruleID). - Execute(); err != nil { - err := shared.ClassifySDKError("delete security group rule", err) - if !IsNotFound(err) { - return err - } - } - } - return nil -} - -func (c *SDKClient) ensureNodeSSHSecurityGroupRule( - ctx context.Context, - nodeSecurityGroupID, bastionSecurityGroupID string, -) error { - if nodeSecurityGroupID == "" || bastionSecurityGroupID == "" { - return fmt.Errorf("%w: node and bastion security group IDs are required", ErrInvalidInput) - } - resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, nodeSecurityGroupID).Execute() - if err != nil { - return shared.ClassifySDKError("list security group rules", err) - } - if hasRemoteSecurityGroupSSHRule(resp.GetItems(), bastionSecurityGroupID) { - return nil - } - protocol := iaas.StringAsCreateProtocol(new(tcpProtocol)) - rule := iaas.NewCreateSecurityGroupRulePayload("ingress") - rule.SetRemoteSecurityGroupId(bastionSecurityGroupID) - rule.SetPortRange(*iaas.NewPortRange(sshPort, sshPort)) - rule.SetProtocol(protocol) - if _, err := c.iaasClient.DefaultAPI.CreateSecurityGroupRule(ctx, c.projectID, c.region, nodeSecurityGroupID). - CreateSecurityGroupRulePayload(*rule). - Execute(); err != nil { - return shared.ClassifySDKError("create security group rule", err) - } - return nil -} - -func (c *SDKClient) findSecurityGroupByTags(ctx context.Context, tags map[string]string) (*SecurityGroup, error) { - matched, err := c.ListSecurityGroupsByTags(ctx, tags) - if err != nil { - return nil, err - } - if len(matched) == 0 { - return nil, fmt.Errorf("%w: no security group matching tags", ErrNotFound) - } - if len(matched) > 1 { - return nil, fmt.Errorf("%w: multiple security groups matching tags", ErrConflict) - } - return matched[0], nil -} - -// addSecurityGroupToServer attaches the group idempotently. Three outcomes are -// expected and must not fail the caller: -// - conflict: already attached -// - invalid input: the API rejects the duplicate ("Duplicate items in the list") -// - not found: the server has no network port yet, so there is nothing to -// attach to — the next reconcile retries once it does -func (c *SDKClient) addSecurityGroupToServer(ctx context.Context, serverID, securityGroupID string) error { - if err := c.iaasClient.DefaultAPI. - AddSecurityGroupToServer(ctx, c.projectID, c.region, serverID, securityGroupID). - Execute(); err != nil { - err := shared.ClassifySDKError("add security group to server", err) - if !IsConflict(err) && !IsInvalidInput(err) && !IsNotFound(err) { - return err - } - } - return nil -} - -func (c *SDKClient) deleteSecurityGroupRules(ctx context.Context, securityGroupID string) error { - resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, securityGroupID).Execute() - if err != nil { - err := shared.ClassifySDKError("list security group rules", err) - if IsNotFound(err) { - return nil - } - return err - } - for _, rule := range resp.GetItems() { - ruleID := rule.GetId() - if ruleID == "" { - continue - } - if err := c.iaasClient.DefaultAPI. - DeleteSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID, ruleID). - Execute(); err != nil { - err := shared.ClassifySDKError("delete security group rule", err) - if !IsNotFound(err) { - return err - } - } - } - return nil -} - -func hasSSHRule(rules []iaas.SecurityGroupRule, cidr string) bool { - for _, rule := range rules { - if rule.GetIpRange() == cidr && isSSHRule(rule) { - return true - } - } - return false -} - -// isSSHRule reports whether the rule is an ingress TCP/22 rule, regardless of -// which CIDR it allows. -func isSSHRule(rule iaas.SecurityGroupRule) bool { - if rule.GetDirection() != "ingress" { - return false - } - portRange := rule.GetPortRange() - if portRange.GetMin() != sshPort || portRange.GetMax() != sshPort { - return false - } - protocol, ok := rule.GetProtocolOk() - return ok && protocol.GetName() == tcpProtocol -} - -func hasRemoteSecurityGroupSSHRule(rules []iaas.SecurityGroupRule, remoteSecurityGroupID string) bool { - for _, rule := range rules { - if rule.GetDirection() != "ingress" || rule.GetRemoteSecurityGroupId() != remoteSecurityGroupID { - continue - } - portRange := rule.GetPortRange() - if portRange.GetMin() != sshPort || portRange.GetMax() != sshPort { - continue - } - if protocol, ok := rule.GetProtocolOk(); ok && protocol.GetName() == tcpProtocol { - return true - } - } - return false -} - func (c *SDKClient) updateAPIServerTargetPool( ctx context.Context, loadBalancerID string, @@ -900,16 +585,6 @@ func loadBalancerFromSDK(in *lb.LoadBalancer) *LoadBalancer { } } -func securityGroupFromSDK(in *iaas.SecurityGroup) *SecurityGroup { - if in == nil { - return nil - } - return &SecurityGroup{ - ID: in.GetId(), - Name: in.GetName(), - } -} - func listenerPort(listeners []lb.Listener) int32 { for _, listener := range listeners { if port := listener.GetPort(); port > 0 { @@ -919,20 +594,6 @@ func listenerPort(listeners []lb.Listener) int32 { return 0 } -func clusterServerSelectorFromTags(tags map[string]string) map[string]string { - selector := map[string]string{} - for _, key := range []string{ - "cluster.x-k8s.io/cluster-name", - "cluster.x-k8s.io/cluster-namespace", - "cluster.x-k8s.io/managed-by", - } { - if value := tags[key]; value != "" { - selector[key] = value - } - } - return selector -} - func isLoadBalancerServiceNotEnabled(err error) bool { return IsUnauthorized(err) && strings.Contains(err.Error(), "Service not enabled") } diff --git a/cloud/sdk_client_test.go b/cloud/sdk_client_test.go index 3e235b7..16eef07 100644 --- a/cloud/sdk_client_test.go +++ b/cloud/sdk_client_test.go @@ -482,10 +482,6 @@ func TestSDKClientEnsureBastionToleratesDuplicateSecurityGroupAttach(t *testing. "labels": map[string]any{"cluster": "test"}, }, }}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/rules"): - writeJSON(t, w, map[string]any{"items": []any{}}) - case r.Method == http.MethodPost && strings.HasSuffix(path, "/rules"): - writeJSON(t, w, map[string]any{"id": "77777777-7777-4777-8777-777777777777", "direction": "ingress"}) case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): writeJSON(t, w, map[string]any{"items": []any{}}) case r.Method == http.MethodPost && strings.HasSuffix(path, "/servers"): @@ -540,162 +536,42 @@ func TestSDKClientEnsureBastionToleratesDuplicateSecurityGroupAttach(t *testing. } } -// TestSDKClientEnsureBastionRevokesRemovedCIDR guards against security-group -// rules only ever being added. Narrowing allowedCIDRs must actually take -// access away from the previously allowed range. -func TestSDKClientEnsureBastionRevokesRemovedCIDR(t *testing.T) { - const staleRuleID = "55555555-5555-4555-8555-555555555555" +func TestSDKClientDeleteBastionDetachesSecurityGroupWithoutDeletingIt(t *testing.T) { var ( - createdRuleCIDRs []string - deletedRuleIDs []string + detachedServerID string + detachedSGID string + deletedServerID string ) server := newSDKTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path := r.URL.Path switch { - case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": testSDKSecurityGroup, "name": "bastion-ssh", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/rules"): - // One pre-existing SSH rule for the CIDR that is being revoked. - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": staleRuleID, - "direction": "ingress", - "ipRange": "198.51.100.0/24", - "portRange": map[string]any{"min": 22, "max": 22}, - "protocol": map[string]any{"name": "tcp"}, - }, - }}) - case r.Method == http.MethodPost && strings.HasSuffix(path, "/rules"): - payload := readJSON(t, r) - if cidr, ok := payload["ipRange"].(string); ok { - createdRuleCIDRs = append(createdRuleCIDRs, cidr) - } - writeJSON(t, w, map[string]any{"id": "77777777-7777-4777-8777-777777777777", "direction": "ingress"}) - case strings.Contains(path, "/security-groups/") && strings.Contains(path, "/servers/"): + case r.Method == http.MethodDelete && strings.Contains(path, "/servers/") && strings.Contains(path, "/security-groups/"): + parts := strings.Split(path, "/servers/") + sub := strings.Split(parts[1], "/security-groups/") + detachedServerID, detachedSGID = sub[0], sub[1] w.WriteHeader(http.StatusNoContent) - case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): - parts := strings.Split(strings.TrimSuffix(path, "/"), "/") - deletedRuleIDs = append(deletedRuleIDs, parts[len(parts)-1]) + case r.Method == http.MethodDelete && strings.HasSuffix(path, "/servers/"+testSDKServerID): + deletedServerID = testSDKServerID w.WriteHeader(http.StatusNoContent) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": testSDKServerID, "name": "bastion", "status": "ACTIVE", "machineType": "c2i.1", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/public-ips"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": "66666666-6666-4666-8666-666666666666", "ip": "203.0.113.10", "networkInterface": "nic-1", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.Contains(path, "/servers/"): - writeJSON(t, w, map[string]any{ - "id": testSDKServerID, "name": "bastion", "status": "ACTIVE", "machineType": "c2i.1", - }) default: t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) } })) client := newTestSDKClient(t, server.URL) - if _, err := client.EnsureBastion(context.Background(), BastionInput{ - Name: "bastion", - ProjectID: testSDKProjectID, - Region: testSDKRegion, - NetworkID: testSDKNetworkID, - ImageID: testSDKImageID, - MachineType: "c2i.1", - SSHKeyName: "default", - AllowedCIDRs: []string{"203.0.113.0/24"}, - Tags: map[string]string{"cluster": "test"}, - }); err != nil { - t.Fatalf("EnsureBastion() error = %v", err) - } - - if len(createdRuleCIDRs) != 1 || createdRuleCIDRs[0] != "203.0.113.0/24" { - t.Fatalf("created rules for %v, want [203.0.113.0/24]", createdRuleCIDRs) + err := client.DeleteBastion( + context.Background(), + BastionInput{Tags: map[string]string{"cluster": "test"}}, + Bastion{ServerID: testSDKServerID, SecurityGroupID: testSDKSecurityGroup}, + ) + if err != nil { + t.Fatalf("DeleteBastion() error = %v", err) } - if len(deletedRuleIDs) != 1 || deletedRuleIDs[0] != staleRuleID { - t.Fatalf("deleted rules %v, want [%s] — the revoked CIDR keeps its SSH access", deletedRuleIDs, staleRuleID) + if detachedServerID != testSDKServerID || detachedSGID != testSDKSecurityGroup { + t.Fatalf("detached (%q, %q), want (%q, %q)", detachedServerID, detachedSGID, testSDKServerID, testSDKSecurityGroup) } -} - -// TestSDKClientEnsureBastionDeduplicatesRepeatedCIDRs guards against a CIDR -// listed twice producing two identical rules: existingRules is a snapshot taken -// before the loop, so the second pass would not see the rule the first pass -// created and the duplicate create fails the whole reconcile. -func TestSDKClientEnsureBastionDeduplicatesRepeatedCIDRs(t *testing.T) { - var createdRuleCIDRs []string - server := newSDKTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - path := r.URL.Path - switch { - case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": testSDKSecurityGroup, "name": "bastion-ssh", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/rules"): - writeJSON(t, w, map[string]any{"items": []any{}}) - case r.Method == http.MethodPost && strings.HasSuffix(path, "/rules"): - payload := readJSON(t, r) - if cidr, ok := payload["ipRange"].(string); ok { - createdRuleCIDRs = append(createdRuleCIDRs, cidr) - } - writeJSON(t, w, map[string]any{"id": "77777777-7777-4777-8777-777777777777", "direction": "ingress"}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": testSDKServerID, "name": "bastion", "status": "ACTIVE", "machineType": "c2i.1", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case strings.Contains(path, "/security-groups/") && strings.Contains(path, "/servers/"): - w.WriteHeader(http.StatusNoContent) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/public-ips"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": "66666666-6666-4666-8666-666666666666", "ip": "203.0.113.10", "networkInterface": "nic-1", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.Contains(path, "/servers/"): - writeJSON(t, w, map[string]any{ - "id": testSDKServerID, "name": "bastion", "status": "ACTIVE", "machineType": "c2i.1", - }) - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) - } - })) - - client := newTestSDKClient(t, server.URL) - if _, err := client.EnsureBastion(context.Background(), BastionInput{ - Name: "bastion", - ProjectID: testSDKProjectID, - Region: testSDKRegion, - NetworkID: testSDKNetworkID, - ImageID: testSDKImageID, - MachineType: "c2i.1", - SSHKeyName: "default", - // The same CIDR twice, plus a distinct one. - AllowedCIDRs: []string{"203.0.113.0/24", "203.0.113.0/24", "198.51.100.0/24"}, - Tags: map[string]string{"cluster": "test"}, - }); err != nil { - t.Fatalf("EnsureBastion() error = %v", err) - } - - if len(createdRuleCIDRs) != 2 { - t.Fatalf("created %d rules (%v), want 2 — the repeated CIDR was created twice", - len(createdRuleCIDRs), createdRuleCIDRs) + if deletedServerID != testSDKServerID { + t.Fatalf("deletedServerID = %q, want %q", deletedServerID, testSDKServerID) } } diff --git a/cloud/services/bastion/bastion.go b/cloud/services/bastion/bastion.go index 80eab63..d89ae2c 100644 --- a/cloud/services/bastion/bastion.go +++ b/cloud/services/bastion/bastion.go @@ -36,6 +36,7 @@ func Input(stackitCluster *infrav1.StackitCluster, cloudInit []byte) cloud.Basti AvailabilityZone: scope.DefaultAvailabilityZone(stackitCluster), SSHKeyName: stackitCluster.Spec.Bastion.SSHKeyName, AllowedCIDRs: stackitCluster.Spec.Bastion.AllowedCIDRs, + SecurityGroupID: stackitCluster.Status.Bastion.SecurityGroupID, Tags: tags, RootVolume: cloud.RootVolumeInput{ SizeGiB: stackitCluster.Spec.Bastion.RootVolume.SizeGiB, @@ -45,9 +46,3 @@ func Input(stackitCluster *infrav1.StackitCluster, cloudInit []byte) cloud.Basti CloudInit: cloudInit, } } - -func NodeSSHAccessTags(stackitCluster *infrav1.StackitCluster) map[string]string { - tags := util.ClusterTags(stackitCluster.Name, stackitCluster.Namespace, stackitCluster.Spec.AdditionalLabels) - tags[util.LabelResourceRole] = util.ResourceRoleNodeSSH - return tags -} diff --git a/cloud/services/securitygroup/reconcile.go b/cloud/services/securitygroup/reconcile.go new file mode 100644 index 0000000..75dbdc5 --- /dev/null +++ b/cloud/services/securitygroup/reconcile.go @@ -0,0 +1,323 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package securitygroup + +import ( + "context" + "fmt" + "slices" + + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" +) + +func (s *Service) Reconcile(ctx context.Context) error { + log := logf.FromContext(ctx) + for _, spec := range s.scope.SecurityGroupSpecs() { + if err := s.reconcileSecurityGroup(ctx, spec); err != nil { + return err + } + } + log.V(1).Info("Reconciled security group resources") + return nil +} + +func (s *Service) Delete(ctx context.Context) error { + log := logf.FromContext(ctx) + for _, spec := range slices.Backward(s.scope.SecurityGroupSpecs()) { + if err := s.deleteSecurityGroup(ctx, spec); err != nil { + return err + } + } + log.V(1).Info("Deleted security group resources") + return nil +} + +// FindByTags finds a single STACKIT security group matching the given tag selector. +func (c *Client) FindByTags(ctx context.Context, tags map[string]string) (*SecurityGroup, error) { + matched, err := c.ListByTags(ctx, tags) + if err != nil { + return nil, err + } + if len(matched) == 0 { + return nil, fmt.Errorf("%w: no security group matching tags", shared.ErrNotFound) + } + if len(matched) > 1 { + return nil, fmt.Errorf("%w: multiple security groups matching tags", shared.ErrConflict) + } + return matched[0], nil +} + +// ListByTags lists STACKIT security groups matching the given tag selector. +func (c *Client) ListByTags(ctx context.Context, tags map[string]string) ([]*SecurityGroup, error) { + if len(tags) == 0 { + return nil, fmt.Errorf("%w: empty tag selector", shared.ErrInvalidInput) + } + resp, err := c.iaasClient.DefaultAPI.ListSecurityGroups(ctx, c.projectID, c.region). + LabelSelector(shared.LabelSelector(tags)). + Execute() + if err != nil { + return nil, shared.ClassifySDKError("list security groups", err) + } + matched := []*SecurityGroup{} + for _, securityGroup := range resp.GetItems() { + if shared.LabelsContainTags(securityGroup.GetLabels(), tags) { + securityGroup := securityGroup + matched = append(matched, securityGroupFromSDK(&securityGroup)) + } + } + return matched, nil +} + +// DeleteByID deletes a security group and its rules by ID, ignoring NotFound errors. +func (c *Client) DeleteByID(ctx context.Context, id string) error { + if id == "" { + return fmt.Errorf("%w: security group ID is empty", shared.ErrInvalidInput) + } + rules, ok, err := c.listRulesIfGroupExists(ctx, id) + if err != nil || !ok { + return err + } + return c.deleteSecurityGroupWithRules(ctx, id, rules) +} + +// AttachToServer attaches a security group to a server idempotently. +// Three error responses are tolerated: +// - conflict: already attached +// - invalid input: duplicate item in security_groups list +// - not found: server has no network port yet +func (c *Client) AttachToServer(ctx context.Context, serverID, securityGroupID string) error { + if err := c.iaasClient.DefaultAPI. + AddSecurityGroupToServer(ctx, c.projectID, c.region, serverID, securityGroupID). + Execute(); err != nil { + err := shared.ClassifySDKError("add security group to server", err) + if !shared.IsConflict(err) && !shared.IsInvalidInput(err) && !shared.IsNotFound(err) { + return err + } + } + logf.FromContext(ctx).V(1).Info("Attached security group to server", "securityGroupID", securityGroupID, "serverID", serverID) + return nil +} + +// DetachFromServer detaches a security group from a server, ignoring NotFound and Conflict errors. +func (c *Client) DetachFromServer(ctx context.Context, serverID, securityGroupID string) error { + if err := c.iaasClient.DefaultAPI. + RemoveSecurityGroupFromServer(ctx, c.projectID, c.region, serverID, securityGroupID). + Execute(); err != nil { + err := shared.ClassifySDKError("remove security group from server", err) + if !shared.IsNotFound(err) && !shared.IsConflict(err) { + return err + } + } + logf.FromContext(ctx).V(1).Info("Detached security group from server", "securityGroupID", securityGroupID, "serverID", serverID) + return nil +} + +func (c *Client) reconcileSecurityGroup(ctx context.Context, spec SecurityGroupSpec) error { + if err := spec.validate(); err != nil { + return err + } + securityGroup, err := c.ensureSecurityGroup(ctx, spec) + if err != nil { + return err + } + if spec.SetID != nil { + spec.SetID(securityGroup.ID) + } + if err := c.ensureSecurityGroupRules(ctx, securityGroup.ID, spec.Rules); err != nil { + return err + } + for _, serverID := range spec.ServerIDs { + if serverID == "" { + continue + } + if err := c.AttachToServer(ctx, serverID, securityGroup.ID); err != nil { + return err + } + } + return nil +} + +func (c *Client) deleteSecurityGroup(ctx context.Context, spec SecurityGroupSpec) error { + if spec.ID == "" && len(spec.Tags) == 0 { + return fmt.Errorf("%w: security group ID or tags are required", shared.ErrInvalidInput) + } + + securityGroupID, rules, err := c.resolveSecurityGroupForDelete(ctx, spec) + if err != nil { + return err + } + if securityGroupID != "" { + if err := c.detachSecurityGroupFromServers(ctx, securityGroupID, spec); err != nil { + return err + } + if err := c.deleteSecurityGroupWithRules(ctx, securityGroupID, rules); err != nil { + return err + } + logf.FromContext(ctx).V(1).Info("Deleted security group", "securityGroupID", securityGroupID) + } + if spec.SetID != nil { + spec.SetID("") + } + return nil +} + +func (c *Client) resolveSecurityGroupForDelete( + ctx context.Context, + spec SecurityGroupSpec, +) (string, []iaas.SecurityGroupRule, error) { + if spec.ID != "" { + rules, ok, err := c.listRulesIfGroupExists(ctx, spec.ID) + if err != nil || ok { + return spec.ID, rules, err + } + } + if len(spec.Tags) == 0 { + return "", nil, nil + } + securityGroup, err := c.FindByTags(ctx, spec.Tags) + if shared.IsNotFound(err) { + return "", nil, nil + } + if err != nil { + return "", nil, err + } + rules, _, err := c.listRulesIfGroupExists(ctx, securityGroup.ID) + return securityGroup.ID, rules, err +} + +func (c *Client) listRulesIfGroupExists(ctx context.Context, id string) ([]iaas.SecurityGroupRule, bool, error) { + rules, err := c.listSecurityGroupRules(ctx, id) + if shared.IsNotFound(err) { + return nil, false, nil + } + if err != nil { + return nil, false, err + } + return rules, true, nil +} + +func (c *Client) ensureSecurityGroup(ctx context.Context, spec SecurityGroupSpec) (*SecurityGroup, error) { + if existing, err := c.FindByTags(ctx, spec.Tags); err == nil { + return existing, nil + } else if !shared.IsNotFound(err) { + return nil, err + } + securityGroup, err := c.iaasClient.DefaultAPI.CreateSecurityGroup(ctx, c.projectID, c.region). + CreateSecurityGroupPayload(spec.toCreatePayload()). + Execute() + if err != nil { + return nil, shared.ClassifySDKError("create security group", err) + } + out := securityGroupFromSDK(securityGroup) + log := logf.FromContext(ctx) + log.V(1).Info("Created security group", "name", spec.Name, "securityGroupID", out.ID) + return out, nil +} + +func (c *Client) deleteSecurityGroupWithRules( + ctx context.Context, + id string, + rules []iaas.SecurityGroupRule, +) error { + for _, rule := range rules { + if err := c.deleteSecurityGroupRule(ctx, id, rule.GetId()); err != nil { + return err + } + } + if err := c.iaasClient.DefaultAPI. + DeleteSecurityGroup(ctx, c.projectID, c.region, id). + Execute(); err != nil { + err := shared.ClassifySDKError("delete security group", err) + if !shared.IsNotFound(err) { + return err + } + } + return nil +} + +func (c *Client) detachSecurityGroupFromServers(ctx context.Context, securityGroupID string, spec SecurityGroupSpec) error { + if err := c.detachServers(ctx, securityGroupID, spec.ServerIDs); err != nil { + return err + } + if len(spec.ServerSelector) == 0 { + return nil + } + serverIDs, err := c.listServerIDsByTags(ctx, spec.ServerSelector) + if err != nil { + return err + } + return c.detachServers(ctx, securityGroupID, serverIDs) +} + +func (c *Client) detachServers(ctx context.Context, securityGroupID string, serverIDs []string) error { + for _, serverID := range serverIDs { + if serverID == "" { + continue + } + if err := c.DetachFromServer(ctx, serverID, securityGroupID); err != nil { + return err + } + } + return nil +} + +func (c *Client) listServerIDsByTags(ctx context.Context, tags map[string]string) ([]string, error) { + resp, err := c.iaasClient.DefaultAPI.ListServers(ctx, c.projectID, c.region). + LabelSelector(shared.LabelSelector(tags)). + Execute() + if err != nil { + return nil, shared.ClassifySDKError("list servers", err) + } + var serverIDs []string + for _, server := range resp.GetItems() { + if shared.LabelsContainTags(server.GetLabels(), tags) && server.GetId() != "" { + serverIDs = append(serverIDs, server.GetId()) + } + } + return serverIDs, nil +} + +func (s SecurityGroupSpec) validate() error { + if s.Name == "" { + return fmt.Errorf("%w: security group name is required", shared.ErrInvalidInput) + } + if len(s.Tags) == 0 { + return fmt.Errorf("%w: security group tags are required", shared.ErrInvalidInput) + } + for _, rule := range s.Rules { + if err := rule.validate(); err != nil { + return err + } + } + return nil +} + +func (s SecurityGroupSpec) toCreatePayload() iaas.CreateSecurityGroupPayload { + payload := iaas.NewCreateSecurityGroupPayload(s.Name) + if s.Description != "" { + payload.SetDescription(s.Description) + } + payload.SetLabels(shared.TagsToSDKLabels(s.Tags)) + return *payload +} + +func securityGroupFromSDK(in *iaas.SecurityGroup) *SecurityGroup { + if in == nil { + return nil + } + return &SecurityGroup{ + ID: in.GetId(), + Name: in.GetName(), + } +} diff --git a/cloud/services/securitygroup/reconcile_test.go b/cloud/services/securitygroup/reconcile_test.go new file mode 100644 index 0000000..3c44ad0 --- /dev/null +++ b/cloud/services/securitygroup/reconcile_test.go @@ -0,0 +1,743 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package securitygroup_test + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + + "github.com/stackitcloud/stackit-sdk-go/core/config" + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" +) + +const ( + testProjectID = "00000000-0000-0000-0000-000000000000" + testRegion = "eu01" + testSecurityGroupID = "11111111-1111-4111-8111-111111111111" + testRemoteSGID = "22222222-2222-4222-8222-222222222222" + testServerID = "33333333-3333-4333-8333-333333333333" + testRuleID1 = "66666666-6666-4666-8666-666666666666" + testRuleID2 = "77777777-7777-4777-8777-777777777777" +) + +type testScope struct { + projectID string + region string + iaasClient *iaas.APIClient + specs []securitygroup.SecurityGroupSpec +} + +func (s *testScope) ProjectID() string { + return s.projectID +} + +func (s *testScope) Region() string { + return s.region +} + +func (s *testScope) IaaSClient() *iaas.APIClient { + return s.iaasClient +} + +func (s *testScope) SecurityGroupSpecs() []securitygroup.SecurityGroupSpec { + return s.specs +} + +func newTestIaaSClient(t *testing.T, handler http.Handler) *iaas.APIClient { + t.Helper() + + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + + iaasClient, err := iaas.NewAPIClient( + config.WithoutAuthentication(), + config.WithEndpoint(server.URL), + ) + if err != nil { + t.Fatalf("iaas.NewAPIClient() error = %v", err) + } + return iaasClient +} + +func newTestService(t *testing.T, specs []securitygroup.SecurityGroupSpec, handler http.Handler) *securitygroup.Service { + t.Helper() + + return securitygroup.New(&testScope{ + projectID: testProjectID, + region: testRegion, + iaasClient: newTestIaaSClient(t, handler), + specs: specs, + }) +} + +func writeJSON(t *testing.T, w http.ResponseWriter, value any) { + t.Helper() + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(value); err != nil { + t.Fatalf("encode response body: %v", err) + } +} + +func readJSON(t *testing.T, r *http.Request) map[string]any { + t.Helper() + var out map[string]any + if err := json.NewDecoder(r.Body).Decode(&out); err != nil { + t.Fatalf("decode request body: %v", err) + } + return out +} + +func TestServiceReconcile(t *testing.T) { + t.Run("creates security group and generic rules when not found", func(t *testing.T) { + var ( + createdSGPayload map[string]any + createdRulePayloads []map[string]any + ) + specs := []securitygroup.SecurityGroupSpec{{ + Name: "cluster-sg", + Description: "Generic security group", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{ + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 6443, + PortMax: 6443, + IPRange: "203.0.113.0/24", + }, + { + Direction: securitygroup.DirectionEgress, + Protocol: securitygroup.ProtocolUDP, + PortMin: 8472, + PortMax: 8472, + RemoteSecurityGroupID: testRemoteSGID, + }, + }, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups"): + createdSGPayload = readJSON(t, r) + writeJSON(t, w, map[string]any{ + "id": testSecurityGroupID, + "name": "cluster-sg", + }) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + createdRulePayloads = append(createdRulePayloads, readJSON(t, r)) + writeJSON(t, w, map[string]any{"id": testRuleID1, "direction": "ingress"}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + + if createdSGPayload["name"] != "cluster-sg" { + t.Fatalf("created SG name = %v, want cluster-sg", createdSGPayload["name"]) + } + if createdSGPayload["description"] != "Generic security group" { + t.Fatalf("created SG description = %v, want Generic security group", createdSGPayload["description"]) + } + labels, _ := createdSGPayload["labels"].(map[string]any) + if labels["cluster"] != "test" { + t.Fatalf("created SG labels = %v, want cluster=test", labels) + } + if len(createdRulePayloads) != 2 { + t.Fatalf("created %d rules, want 2", len(createdRulePayloads)) + } + if createdRulePayloads[0]["direction"] != "ingress" || createdRulePayloads[0]["ipRange"] != "203.0.113.0/24" { + t.Fatalf("first rule payload = %#v", createdRulePayloads[0]) + } + if createdRulePayloads[1]["direction"] != "egress" || createdRulePayloads[1]["remoteSecurityGroupId"] != testRemoteSGID { + t.Fatalf("second rule payload = %#v", createdRulePayloads[1]) + } + }) + + t.Run("reuses existing security group found by tags without creating duplicate", func(t *testing.T) { + specs := []securitygroup.SecurityGroupSpec{{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testSecurityGroupID, + "name": "cluster-sg", + "labels": map[string]any{"cluster": "test"}, + }, + }}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{}}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + }) + + t.Run("returns ErrConflict when multiple security groups match tags", func(t *testing.T) { + specs := []securitygroup.SecurityGroupSpec{{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testSecurityGroupID, "name": "sg-1", "labels": map[string]any{"cluster": "test"}}, + map[string]any{"id": testRemoteSGID, "name": "sg-2", "labels": map[string]any{"cluster": "test"}}, + }}) + })) + + err := svc.Reconcile(context.Background()) + if !errors.Is(err, shared.ErrConflict) { + t.Fatalf("Reconcile() error = %v, want ErrConflict", err) + } + }) +} + +func TestServiceReconcileBastionAndNodeSSH(t *testing.T) { + var ( + bastionSGID string + bastionCreated bool + nodeRulesCreated []map[string]any + attachedServerIDs []string + ) + specs := []securitygroup.SecurityGroupSpec{ + { + Name: "cluster-bastion-ssh", + Tags: map[string]string{"role": "bastion"}, + SetID: func(id string) { + bastionSGID = id + }, + Rules: []securitygroup.Rule{ + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + IPRange: "203.0.113.0/24", + }, + { + Direction: securitygroup.DirectionEgress, + }, + }, + }, + { + Name: "cluster-node-ssh", + Tags: map[string]string{"role": "node-ssh"}, + ServerIDs: []string{testServerID}, + Rules: []securitygroup.Rule{ + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + RemoteSecurityGroupTags: map[string]string{"role": "bastion"}, + }, + { + Direction: securitygroup.DirectionEgress, + }, + }, + }, + } + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + selector := r.URL.Query().Get("label_selector") + if bastionCreated && strings.Contains(selector, "role=bastion") { + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testRemoteSGID, + "name": "cluster-bastion-ssh", + "labels": map[string]any{"role": "bastion"}, + }, + }}) + return + } + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups"): + payload := readJSON(t, r) + if payload["name"] == "cluster-bastion-ssh" { + bastionCreated = true + writeJSON(t, w, map[string]any{"id": testRemoteSGID, "name": "cluster-bastion-ssh"}) + return + } + writeJSON(t, w, map[string]any{"id": testSecurityGroupID, "name": "cluster-node-ssh"}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testRemoteSGID+"/rules"): + // Simulate STACKIT default egress rule already present. + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRuleID1, "direction": "egress", "ethertype": "IPv4"}, + }}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups/"+testRemoteSGID+"/rules"): + writeJSON(t, w, map[string]any{"id": testRuleID2, "direction": "ingress"}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + nodeRulesCreated = append(nodeRulesCreated, readJSON(t, r)) + writeJSON(t, w, map[string]any{"id": testRuleID1, "direction": "ingress"}) + case r.Method == http.MethodPut && strings.HasSuffix(path, "/servers/"+testServerID+"/security-groups/"+testSecurityGroupID): + attachedServerIDs = append(attachedServerIDs, testServerID) + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + if bastionSGID != testRemoteSGID { + t.Fatalf("bastionSGID = %q, want %q", bastionSGID, testRemoteSGID) + } + if len(nodeRulesCreated) != 2 { + t.Fatalf("nodeRulesCreated = %#v, want 2 rules (ingress from bastion + egress)", nodeRulesCreated) + } + if nodeRulesCreated[0]["remoteSecurityGroupId"] != testRemoteSGID { + t.Fatalf("node ingress rule remoteSecurityGroupId = %v, want %q", nodeRulesCreated[0]["remoteSecurityGroupId"], testRemoteSGID) + } + if nodeRulesCreated[1]["direction"] != "egress" { + t.Fatalf("node egress rule direction = %v, want egress", nodeRulesCreated[1]["direction"]) + } + if !slices.Equal(attachedServerIDs, []string{testServerID}) { + t.Fatalf("attachedServerIDs = %v, want [%s]", attachedServerIDs, testServerID) + } +} + +func TestServiceReconcileRules(t *testing.T) { + var ( + createdRules []map[string]any + deletedRuleIDs []string + ) + specs := []securitygroup.SecurityGroupSpec{{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{ + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 443, + PortMax: 443, + IPRange: "203.0.113.0/24", + }, + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 443, + PortMax: 443, + IPRange: "203.0.113.0/24", + }, + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 10250, + PortMax: 10250, + RemoteSecurityGroupID: testRemoteSGID, + }, + }, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testSecurityGroupID, + "name": "cluster-sg", + "labels": map[string]any{"cluster": "test"}, + }, + }}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testRuleID1, + "direction": "ingress", + "remoteSecurityGroupId": testRemoteSGID, + "portRange": map[string]any{"min": 10250, "max": 10250}, + "protocol": map[string]any{"name": "tcp"}, + }, + map[string]any{ + "id": testRuleID2, + "direction": "ingress", + "ipRange": "198.51.100.0/24", + "portRange": map[string]any{"min": 22, "max": 22}, + "protocol": map[string]any{"name": "tcp"}, + }, + }}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + createdRules = append(createdRules, readJSON(t, r)) + writeJSON(t, w, map[string]any{"id": "88888888-8888-4888-8888-888888888888", "direction": "ingress"}) + case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): + parts := strings.Split(strings.TrimSuffix(path, "/"), "/") + deletedRuleIDs = append(deletedRuleIDs, parts[len(parts)-1]) + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + + if len(createdRules) != 1 || createdRules[0]["ipRange"] != "203.0.113.0/24" { + t.Fatalf("createdRules = %#v, want single rule for 203.0.113.0/24", createdRules) + } + if !slices.Equal(deletedRuleIDs, []string{testRuleID2}) { + t.Fatalf("deletedRuleIDs = %v, want [%s]", deletedRuleIDs, testRuleID2) + } +} + +func TestServiceReconcileValidation(t *testing.T) { + tests := []struct { + name string + spec securitygroup.SecurityGroupSpec + }{ + { + name: "missing name", + spec: securitygroup.SecurityGroupSpec{ + Tags: map[string]string{"cluster": "test"}, + }, + }, + { + name: "missing tags", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + }, + }, + { + name: "invalid rule direction", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{Direction: "invalid", Protocol: securitygroup.ProtocolTCP, PortMin: 80, PortMax: 80, IPRange: "0.0.0.0/0"}}, + }, + }, + { + name: "both ipRange and remoteSecurityGroupID set", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 80, + PortMax: 80, + IPRange: "0.0.0.0/0", + RemoteSecurityGroupID: testRemoteSGID, + }}, + }, + }, + { + name: "both ipRange and remoteSecurityGroupTags set", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 80, + PortMax: 80, + IPRange: "0.0.0.0/0", + RemoteSecurityGroupTags: map[string]string{"role": "bastion"}, + }}, + }, + }, + { + name: "invalid port range", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{Direction: securitygroup.DirectionIngress, Protocol: securitygroup.ProtocolTCP, PortMin: 8080, PortMax: 80, IPRange: "0.0.0.0/0"}}, + }, + }, + { + name: "port range without protocol", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{Direction: securitygroup.DirectionIngress, PortMin: 80, PortMax: 80, IPRange: "0.0.0.0/0"}}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc := newTestService(t, []securitygroup.SecurityGroupSpec{tt.spec}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("unexpected request before validation failure: %s %s", r.Method, r.URL.String()) + })) + + err := svc.Reconcile(context.Background()) + if !errors.Is(err, shared.ErrInvalidInput) { + t.Fatalf("Reconcile() error = %v, want ErrInvalidInput", err) + } + }) + } +} + +func TestServiceDelete(t *testing.T) { + t.Run("deletes rules and security group using ID when provided", func(t *testing.T) { + var ( + deletedRules []string + deletedSGID string + ) + specs := []securitygroup.SecurityGroupSpec{{ + ID: testSecurityGroupID, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRuleID1, "direction": "ingress"}, + map[string]any{"id": testRuleID2, "direction": "egress"}, + }}) + case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): + parts := strings.Split(strings.TrimSuffix(path, "/"), "/") + deletedRules = append(deletedRules, parts[len(parts)-1]) + w.WriteHeader(http.StatusNoContent) + case r.Method == http.MethodDelete && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID): + deletedSGID = testSecurityGroupID + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Delete(context.Background()); err != nil { + t.Fatalf("Delete() error = %v", err) + } + if !slices.Equal(deletedRules, []string{testRuleID1, testRuleID2}) { + t.Fatalf("deletedRules = %v, want [%s %s]", deletedRules, testRuleID1, testRuleID2) + } + if deletedSGID != testSecurityGroupID { + t.Fatalf("deletedSGID = %q, want %q", deletedSGID, testSecurityGroupID) + } + }) + + t.Run("deletes rules and security group found by tags", func(t *testing.T) { + var ( + deletedRules []string + deletedSGID string + ) + specs := []securitygroup.SecurityGroupSpec{{ + Tags: map[string]string{"cluster": "test"}, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testSecurityGroupID, "name": "cluster-sg", "labels": map[string]any{"cluster": "test"}}, + }}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRuleID1, "direction": "ingress"}, + map[string]any{"id": testRuleID2, "direction": "egress"}, + }}) + case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): + parts := strings.Split(strings.TrimSuffix(path, "/"), "/") + deletedRules = append(deletedRules, parts[len(parts)-1]) + w.WriteHeader(http.StatusNoContent) + case r.Method == http.MethodDelete && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID): + deletedSGID = testSecurityGroupID + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Delete(context.Background()); err != nil { + t.Fatalf("Delete() error = %v", err) + } + if !slices.Equal(deletedRules, []string{testRuleID1, testRuleID2}) { + t.Fatalf("deletedRules = %v, want [%s %s]", deletedRules, testRuleID1, testRuleID2) + } + if deletedSGID != testSecurityGroupID { + t.Fatalf("deletedSGID = %q, want %q", deletedSGID, testSecurityGroupID) + } + }) + + t.Run("succeeds when security group does not exist", func(t *testing.T) { + specs := []securitygroup.SecurityGroupSpec{{ + Tags: map[string]string{"cluster": "test"}, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, map[string]any{"items": []any{}}) + })) + + if err := svc.Delete(context.Background()); err != nil { + t.Fatalf("Delete() error = %v, want nil", err) + } + }) + + t.Run("returns ErrInvalidInput when tags are empty", func(t *testing.T) { + svc := newTestService(t, []securitygroup.SecurityGroupSpec{{}}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + })) + + err := svc.Delete(context.Background()) + if !errors.Is(err, shared.ErrInvalidInput) { + t.Fatalf("Delete() error = %v, want ErrInvalidInput", err) + } + }) +} + +func TestServiceDeleteStaleIDFallbackAndDetach(t *testing.T) { + var ( + detachedPairs []string + deletedSGIDs []string + ) + specs := []securitygroup.SecurityGroupSpec{ + { + ID: "99999999-9999-4999-8999-999999999999", + Tags: map[string]string{"role": "bastion"}, + }, + { + Tags: map[string]string{"role": "node-ssh"}, + ServerSelector: map[string]string{"cluster": "test"}, + }, + } + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/99999999-9999-4999-8999-999999999999/rules"): + w.WriteHeader(http.StatusNotFound) + writeJSON(t, w, map[string]any{"message": "not found"}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + selector := r.URL.Query().Get("label_selector") + if strings.Contains(selector, "role=node-ssh") { + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testSecurityGroupID, "name": "node-ssh", "labels": map[string]any{"role": "node-ssh"}}, + }}) + return + } + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRemoteSGID, "name": "bastion-ssh", "labels": map[string]any{"role": "bastion"}}, + }}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testServerID, + "name": "worker-1", + "machineType": "c2i.1", + "labels": map[string]any{"cluster": "test"}, + }, + }}) + case r.Method == http.MethodDelete && strings.Contains(path, "/servers/") && strings.Contains(path, "/security-groups/"): + parts := strings.Split(path, "/servers/") + sub := strings.Split(parts[1], "/security-groups/") + detachedPairs = append(detachedPairs, sub[0]+":"+sub[1]) + w.WriteHeader(http.StatusNoContent) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/rules"): + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodDelete && strings.Contains(path, "/security-groups/"): + id := path[strings.LastIndex(path, "/security-groups/")+len("/security-groups/"):] + deletedSGIDs = append(deletedSGIDs, id) + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Delete(context.Background()); err != nil { + t.Fatalf("Delete() error = %v", err) + } + if !slices.Equal(detachedPairs, []string{testServerID + ":" + testSecurityGroupID}) { + t.Fatalf("detachedPairs = %v, want [%s:%s]", detachedPairs, testServerID, testSecurityGroupID) + } + if !slices.Equal(deletedSGIDs, []string{testSecurityGroupID, testRemoteSGID}) { + t.Fatalf("deletedSGIDs = %v, want [%s %s] (reverse order: node-ssh then bastion-ssh)", deletedSGIDs, testSecurityGroupID, testRemoteSGID) + } +} + +func TestFindByTagsAndDeleteByID(t *testing.T) { + t.Run("FindByTags returns matching security group", func(t *testing.T) { + svc := newTestService(t, nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testSecurityGroupID, "name": "bastion-ssh", "labels": map[string]any{"cluster": "test"}}, + }}) + })) + + got, err := svc.FindByTags(context.Background(), map[string]string{"cluster": "test"}) + if err != nil { + t.Fatalf("FindByTags() error = %v", err) + } + if got.ID != testSecurityGroupID { + t.Fatalf("FindByTags().ID = %q, want %q", got.ID, testSecurityGroupID) + } + }) + + t.Run("DeleteByID deletes rules and security group and ignores NotFound", func(t *testing.T) { + var ( + deletedRules []string + deletedSGID string + ) + svc := newTestService(t, nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRuleID1, "direction": "ingress"}, + }}) + case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): + parts := strings.Split(strings.TrimSuffix(path, "/"), "/") + deletedRules = append(deletedRules, parts[len(parts)-1]) + w.WriteHeader(http.StatusNoContent) + case r.Method == http.MethodDelete && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID): + deletedSGID = testSecurityGroupID + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.DeleteByID(context.Background(), testSecurityGroupID); err != nil { + t.Fatalf("DeleteByID() error = %v", err) + } + if !slices.Equal(deletedRules, []string{testRuleID1}) { + t.Fatalf("deletedRules = %v, want [%s]", deletedRules, testRuleID1) + } + if deletedSGID != testSecurityGroupID { + t.Fatalf("deletedSGID = %q, want %q", deletedSGID, testSecurityGroupID) + } + }) +} diff --git a/cloud/services/securitygroup/rules.go b/cloud/services/securitygroup/rules.go new file mode 100644 index 0000000..901f585 --- /dev/null +++ b/cloud/services/securitygroup/rules.go @@ -0,0 +1,198 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package securitygroup + +import ( + "context" + "fmt" + "slices" + "strings" + + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" +) + +func (c *Client) ensureSecurityGroupRules(ctx context.Context, securityGroupID string, rules []Rule) error { + if securityGroupID == "" { + return fmt.Errorf("%w: security group ID is required", shared.ErrInvalidInput) + } + existingRules, err := c.listSecurityGroupRules(ctx, securityGroupID) + if err != nil { + return err + } + desired := make([]Rule, 0, len(rules)) + for _, rule := range rules { + rule, err := c.resolveRule(ctx, rule) + if err != nil { + return err + } + if slices.ContainsFunc(desired, rule.equal) { + continue + } + desired = append(desired, rule) + if hasRule(existingRules, rule) { + continue + } + if err := c.createSecurityGroupRule(ctx, securityGroupID, rule); err != nil { + return err + } + } + + for _, existing := range existingRules { + if slices.ContainsFunc(desired, ruleFromSDK(existing).equal) { + continue + } + if err := c.deleteSecurityGroupRule(ctx, securityGroupID, existing.GetId()); err != nil { + return err + } + } + return nil +} + +func (c *Client) resolveRule(ctx context.Context, rule Rule) (Rule, error) { + rule = rule.normalize() + if rule.RemoteSecurityGroupID == "" && len(rule.RemoteSecurityGroupTags) > 0 { + remoteSG, err := c.FindByTags(ctx, rule.RemoteSecurityGroupTags) + if err != nil { + return Rule{}, err + } + rule.RemoteSecurityGroupID = remoteSG.ID + } + rule.RemoteSecurityGroupTags = nil + return rule, nil +} + +func (c *Client) createSecurityGroupRule(ctx context.Context, securityGroupID string, rule Rule) error { + if _, err := c.iaasClient.DefaultAPI.CreateSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID). + CreateSecurityGroupRulePayload(rule.toCreatePayload()). + Execute(); err != nil { + return shared.ClassifySDKError("create security group rule", err) + } + return nil +} + +func (c *Client) listSecurityGroupRules(ctx context.Context, securityGroupID string) ([]iaas.SecurityGroupRule, error) { + resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, securityGroupID).Execute() + if err != nil { + return nil, shared.ClassifySDKError("list security group rules", err) + } + return resp.GetItems(), nil +} + +func (c *Client) deleteSecurityGroupRule(ctx context.Context, securityGroupID, ruleID string) error { + if ruleID == "" { + return nil + } + if err := c.iaasClient.DefaultAPI. + DeleteSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID, ruleID). + Execute(); err != nil { + err := shared.ClassifySDKError("delete security group rule", err) + if !shared.IsNotFound(err) { + return err + } + } + return nil +} + +func (r Rule) validate() error { + if r.Direction != DirectionIngress && r.Direction != DirectionEgress { + return fmt.Errorf( + "%w: security group rule direction must be %q or %q", + shared.ErrInvalidInput, + DirectionIngress, + DirectionEgress, + ) + } + if r.IPRange != "" && (r.RemoteSecurityGroupID != "" || len(r.RemoteSecurityGroupTags) > 0) { + return fmt.Errorf( + "%w: security group rule cannot specify both ipRange and remoteSecurityGroupID", + shared.ErrInvalidInput, + ) + } + if r.hasInvalidPortRange() { + return fmt.Errorf("%w: invalid security group rule port range", shared.ErrInvalidInput) + } + if r.hasPortRange() && r.Protocol == "" { + return fmt.Errorf("%w: security group rule port range requires a protocol", shared.ErrInvalidInput) + } + return nil +} + +func (r Rule) hasPortRange() bool { + return r.PortMin > 0 || r.PortMax > 0 +} + +func (r Rule) hasInvalidPortRange() bool { + return r.PortMin < 0 || r.PortMax < 0 || r.PortMin > r.PortMax +} + +func (p Protocol) normalize() Protocol { + return Protocol(strings.ToLower(string(p))) +} + +func (r Rule) normalize() Rule { + r.Protocol = r.Protocol.normalize() + return r +} + +func (r Rule) equal(other Rule) bool { + return r.Direction == other.Direction && + r.Protocol == other.Protocol && + r.PortMin == other.PortMin && + r.PortMax == other.PortMax && + r.IPRange == other.IPRange && + r.RemoteSecurityGroupID == other.RemoteSecurityGroupID +} + +func (r Rule) toCreatePayload() iaas.CreateSecurityGroupRulePayload { + payload := iaas.NewCreateSecurityGroupRulePayload(string(r.Direction)) + if r.Protocol != "" { + protocol := iaas.StringAsCreateProtocol(new(string(r.Protocol))) + payload.SetProtocol(protocol) + } + if r.hasPortRange() { + payload.SetPortRange(*iaas.NewPortRange(r.PortMin, r.PortMax)) + } + if r.IPRange != "" { + payload.SetIpRange(r.IPRange) + } + if r.RemoteSecurityGroupID != "" { + payload.SetRemoteSecurityGroupId(r.RemoteSecurityGroupID) + } + return *payload +} + +func hasRule(rules []iaas.SecurityGroupRule, desired Rule) bool { + return slices.ContainsFunc(rules, func(rule iaas.SecurityGroupRule) bool { + return ruleFromSDK(rule).equal(desired) + }) +} + +func ruleFromSDK(in iaas.SecurityGroupRule) Rule { + var ruleProtocol Protocol + if protocol, ok := in.GetProtocolOk(); ok { + ruleProtocol = Protocol(protocol.GetName()).normalize() + } + var portMin, portMax int64 + if portRange, ok := in.GetPortRangeOk(); ok { + portMin = portRange.GetMin() + portMax = portRange.GetMax() + } + return Rule{ + Direction: Direction(in.GetDirection()), + Protocol: ruleProtocol, + PortMin: portMin, + PortMax: portMax, + IPRange: in.GetIpRange(), + RemoteSecurityGroupID: in.GetRemoteSecurityGroupId(), + } +} diff --git a/cloud/services/securitygroup/service.go b/cloud/services/securitygroup/service.go new file mode 100644 index 0000000..584227a --- /dev/null +++ b/cloud/services/securitygroup/service.go @@ -0,0 +1,102 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package securitygroup + +import iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + +// Direction specifies the traffic direction of a security group rule. +type Direction string + +const ( + DirectionIngress Direction = "ingress" + DirectionEgress Direction = "egress" +) + +// Protocol specifies the network protocol of a security group rule. +type Protocol string + +const ( + ProtocolTCP Protocol = "tcp" + ProtocolUDP Protocol = "udp" + ProtocolICMP Protocol = "icmp" +) + +// SecurityGroup describes a STACKIT security group. +type SecurityGroup struct { + ID string + Name string +} + +// Rule describes a desired STACKIT security group rule. +type Rule struct { + Direction Direction + Protocol Protocol + PortMin int64 + PortMax int64 + IPRange string + RemoteSecurityGroupID string + RemoteSecurityGroupTags map[string]string +} + +// SecurityGroupSpec describes the desired state of a STACKIT security group and its rules. +type SecurityGroupSpec struct { + ID string + Name string + Description string + Tags map[string]string + Rules []Rule + ServerIDs []string + ServerSelector map[string]string + SetID func(string) +} + +// ClientScope is an interface that provides the IaaS client and target scope for security group operations. +type ClientScope interface { + ProjectID() string + Region() string + IaaSClient() *iaas.APIClient +} + +// Scope is an interface that provides scope data needed for security group reconciliation. +type Scope interface { + ClientScope + SecurityGroupSpecs() []SecurityGroupSpec +} + +// Client provides direct security group operations against the STACKIT IaaS API. +type Client struct { + iaasClient *iaas.APIClient + projectID string + region string +} + +// NewClient returns a Client for the given scope. +func NewClient(scope ClientScope) *Client { + return &Client{ + iaasClient: scope.IaaSClient(), + projectID: scope.ProjectID(), + region: scope.Region(), + } +} + +// Service implements security groups reconciler. +type Service struct { + scope Scope + *Client +} + +// New returns Service from given scope. +func New(scope Scope) *Service { + return &Service{ + scope: scope, + Client: NewClient(scope), + } +} diff --git a/cloud/types.go b/cloud/types.go index 6cca29a..98b9fda 100644 --- a/cloud/types.go +++ b/cloud/types.go @@ -15,6 +15,7 @@ package cloud import ( "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" ) @@ -44,10 +45,7 @@ type LoadBalancer struct { type PublicIP = publicip.PublicIP // SecurityGroup describes a STACKIT security group. -type SecurityGroup struct { - ID string - Name string -} +type SecurityGroup = securitygroup.SecurityGroup // Bastion describes the provider-managed SSH bastion resources. type Bastion struct { @@ -84,20 +82,12 @@ type BastionInput struct { AvailabilityZone string SSHKeyName string AllowedCIDRs []string + SecurityGroupID string Tags map[string]string RootVolume RootVolumeInput CloudInit []byte } -// NodeSSHAccessInput holds parameters required to allow SSH from the bastion -// security group to cluster nodes. -type NodeSSHAccessInput struct { - Name string - ServerID string - BastionSecurityGroupID string - Tags map[string]string -} - // RootVolumeInput describes the root disk of a VM. type RootVolumeInput struct { SizeGiB int diff --git a/controller/stackitcluster_bastion.go b/controller/stackitcluster_bastion.go index e2c1a1a..8c7d888 100644 --- a/controller/stackitcluster_bastion.go +++ b/controller/stackitcluster_bastion.go @@ -30,6 +30,7 @@ import ( "github.com/stackitcloud/cluster-api-provider-stackit/cloud" bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/scope" ) @@ -115,13 +116,21 @@ func (r *StackitClusterReconciler) reconcileBastion( return ctrl.Result{RequeueAfter: retryableErrorRequeueAfter}, false, nil } - hadBastionStatus := hasBastionStatus(stackitCluster.Status.Bastion) + hadBastionServer := stackitCluster.Status.Bastion.ServerID != "" + // Temporary ordering until bastion server reconciliation is extracted into a + // service: ensure the security group first so its ID can be attached to the + // bastion server by EnsureBastion. + if err := securitygroup.New(clusterScope).Reconcile(ctx); err != nil { + return ctrl.Result{}, false, err + } + input.SecurityGroupID = stackitCluster.Status.Bastion.SecurityGroupID + bastion, err := cloudClient.EnsureBastion(ctx, input) if err != nil { return ctrl.Result{}, false, err } clusterScope.SetBastionStatus(bastion, bastionCloudInitHash(cloudInit)) - if !hadBastionStatus { + if !hadBastionServer { record.Eventf( stackitCluster, "BastionCreated", "Created bastion %s", bastion.ServerID, ) @@ -192,17 +201,20 @@ func (r *StackitClusterReconciler) deleteBastionResources( status cloud.Bastion, ignoreNotFound bool, ) error { - if err := cloudClient.DeleteNodeSSHAccess(ctx, bastionservice.NodeSSHAccessTags(clusterScope.StackitCluster)); err != nil { + if err := publicip.New(clusterScope).Delete(ctx); err != nil { if !ignoreNotFound || !cloud.IsNotFound(err) { return err } } - if err := publicip.New(clusterScope).Delete(ctx); err != nil { + if err := cloudClient.DeleteBastion(ctx, input, status); err != nil { if !ignoreNotFound || !cloud.IsNotFound(err) { return err } } - if err := cloudClient.DeleteBastion(ctx, input, status); err != nil { + // Temporary ordering until bastion server reconciliation is extracted into a + // service: the security group must be deleted after the server is detached + // and deleted by DeleteBastion. + if err := securitygroup.New(clusterScope).Delete(ctx); err != nil { if !ignoreNotFound || !cloud.IsNotFound(err) { return err } @@ -211,7 +223,7 @@ func (r *StackitClusterReconciler) deleteBastionResources( } func bastionNeedsRecreate(stackitCluster *infrav1.StackitCluster, cloudInit []byte) bool { - if !hasBastionStatus(stackitCluster.Status.Bastion) { + if stackitCluster.Status.Bastion.ServerID == "" { return false } return stackitCluster.Status.Bastion.CloudInitHash != bastionCloudInitHash(cloudInit) diff --git a/controller/stackitcluster_controller_test.go b/controller/stackitcluster_controller_test.go index ccd6cf1..3898f6d 100644 --- a/controller/stackitcluster_controller_test.go +++ b/controller/stackitcluster_controller_test.go @@ -28,7 +28,9 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" cloudfake "github.com/stackitcloud/cluster-api-provider-stackit/cloud/fake" - bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" + "github.com/stackitcloud/cluster-api-provider-stackit/scope" + "github.com/stackitcloud/cluster-api-provider-stackit/util" ) var _ = Describe("StackitCluster Controller", func() { @@ -135,7 +137,34 @@ var _ = Describe("StackitCluster Controller", func() { Expect(got.Status.Bastion.SecurityGroupID).NotTo(BeEmpty()) Expect(fakeCloud.ServerCount()).To(Equal(1)) Expect(fakeCloud.PublicIPCount()).To(Equal(1)) - Expect(fakeCloud.SecurityGroupCount()).To(Equal(1)) + Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) + Expect(fakeCloud.SecurityGroupRules(got.Status.Bastion.SecurityGroupID)).To(ConsistOf( + securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + IPRange: "203.0.113.10/32", + }, + securitygroup.Rule{ + Direction: securitygroup.DirectionEgress, + }, + )) + nodeSGs, err := fakeCloud.ListSecurityGroupsByTags(ctx, scope.NodeSSHTags(got)) + Expect(err).NotTo(HaveOccurred()) + Expect(nodeSGs).To(HaveLen(1)) + Expect(fakeCloud.SecurityGroupRules(nodeSGs[0].ID)).To(ConsistOf( + securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + RemoteSecurityGroupID: got.Status.Bastion.SecurityGroupID, + }, + securitygroup.Rule{ + Direction: securitygroup.DirectionEgress, + }, + )) expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionTrue, "Available") }) @@ -169,14 +198,18 @@ var _ = Describe("StackitCluster Controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) - _, err = fakeCloud.EnsureNodeSSHAccess(ctx, cloud.NodeSSHAccessInput{ - Name: got.Name + "-node-ssh", - ServerID: got.Status.Bastion.ServerID, - BastionSecurityGroupID: got.Status.Bastion.SecurityGroupID, - Tags: bastionservice.NodeSSHAccessTags(got), + Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) + nodeSGs, err := fakeCloud.ListSecurityGroupsByTags(ctx, scope.NodeSSHTags(got)) + Expect(err).NotTo(HaveOccurred()) + Expect(nodeSGs).To(HaveLen(1)) + workerServer, err := fakeCloud.CreateServer(ctx, cloud.CreateServerInput{ + Name: got.Name + "-worker-0", + Tags: util.MachineTags(got.Name, got.Namespace, "worker-0", "uid-1", nil), + SecurityGroups: []string{nodeSGs[0].ID}, }) Expect(err).NotTo(HaveOccurred()) - Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) + Expect(fakeCloud.ServerHasSecurityGroup(workerServer.ID, nodeSGs[0].ID)).To(BeTrue()) + got.Spec.Bastion.Enabled = false Expect(k8sClient.Update(ctx, got)).To(Succeed()) @@ -186,7 +219,8 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(got.Status.Bastion).To(Equal(infrav1.StackitBastionStatus{})) - Expect(fakeCloud.ServerCount()).To(Equal(0)) + Expect(fakeCloud.ServerHasSecurityGroup(workerServer.ID, nodeSGs[0].ID)).To(BeFalse()) + Expect(fakeCloud.ServerCount()).To(Equal(1)) Expect(fakeCloud.PublicIPCount()).To(Equal(0)) Expect(fakeCloud.SecurityGroupCount()).To(Equal(0)) expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionTrue, "Skipped") @@ -202,13 +236,6 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(got.Status.Bastion.PublicIP).NotTo(BeEmpty()) - _, err = fakeCloud.EnsureNodeSSHAccess(ctx, cloud.NodeSSHAccessInput{ - Name: got.Name + "-node-ssh", - ServerID: got.Status.Bastion.ServerID, - BastionSecurityGroupID: got.Status.Bastion.SecurityGroupID, - Tags: bastionservice.NodeSSHAccessTags(got), - }) - Expect(err).NotTo(HaveOccurred()) Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) Expect(k8sClient.Delete(ctx, got)).To(Succeed()) @@ -450,13 +477,6 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) oldServerID := got.Status.Bastion.ServerID Expect(oldServerID).NotTo(BeEmpty()) - _, err = fakeCloud.EnsureNodeSSHAccess(ctx, cloud.NodeSSHAccessInput{ - Name: got.Name + "-node-ssh", - ServerID: got.Status.Bastion.ServerID, - BastionSecurityGroupID: got.Status.Bastion.SecurityGroupID, - Tags: bastionservice.NodeSSHAccessTags(got), - }) - Expect(err).NotTo(HaveOccurred()) Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) configMap := &corev1.ConfigMap{} diff --git a/controller/stackitcluster_infrastructure.go b/controller/stackitcluster_infrastructure.go index f2c55e8..318c18d 100644 --- a/controller/stackitcluster_infrastructure.go +++ b/controller/stackitcluster_infrastructure.go @@ -34,6 +34,7 @@ import ( bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" loadbalancerservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/loadbalancer" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/scope" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -385,8 +386,9 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS } // Cleanup runs unconditionally: neither spec nor status is a trustworthy - // record of what exists in the cloud. ResolveID, DeleteBastion and - // DeleteNodeSSHAccess fall back to tag lookups and tolerate NotFound. + // record of what exists in the cloud. ResolveID, DeleteBastion, and the + // public IP and security group services fall back to tag lookups and + // tolerate NotFound. cloudClient, err := util.BuildCloudClient(ctx, r.Client, r.CloudClientFactory, stackitCluster) if err != nil { // A missing credentials Secret cannot be recovered from and commonly @@ -435,12 +437,6 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS "Deleted API server load balancer %s", loadBalancerID, ) } - if err := cloudClient.DeleteNodeSSHAccess(ctx, bastionservice.NodeSSHAccessTags(stackitCluster)); err != nil && !cloud.IsNotFound(err) { - log.Error(err, "Failed to delete bastion node SSH access", "cluster", stackitCluster.Name) - record.Warnf(stackitCluster, "FailedDeleteNodeSSHAccess", - "Failed to delete bastion node SSH access: %v", err) - return ctrl.Result{}, err - } hadBastion := hasBastionStatus(stackitCluster.Status.Bastion) if err := publicip.New(clusterScope).Delete(ctx); err != nil && !cloud.IsNotFound(err) { log.Error(err, "Failed to delete bastion public IP", "cluster", stackitCluster.Name) @@ -457,6 +453,12 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS "Failed to delete bastion: %v", err) return ctrl.Result{}, err } + if err := securitygroup.New(clusterScope).Delete(ctx); err != nil && !cloud.IsNotFound(err) { + log.Error(err, "Failed to delete bastion security groups", "cluster", stackitCluster.Name) + record.Warnf(stackitCluster, "FailedDeleteBastion", + "Failed to delete bastion security groups: %v", err) + return ctrl.Result{}, err + } if hadBastion { clusterScope.ClearBastionStatus() record.Event(stackitCluster, "BastionDeleted", "Deleted bastion") diff --git a/controller/stackitmachine_controller_test.go b/controller/stackitmachine_controller_test.go index 0efd701..85b1b39 100644 --- a/controller/stackitmachine_controller_test.go +++ b/controller/stackitmachine_controller_test.go @@ -28,6 +28,7 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" cloudfake "github.com/stackitcloud/cluster-api-provider-stackit/cloud/fake" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -237,7 +238,7 @@ var _ = Describe("StackitMachine Controller", func() { got := &infrav1.StackitMachine{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(got.Status.Ready).To(BeTrue()) - Expect(fakeCloud.EnsureNodeSSHCalls).To(Equal(1)) + Expect(fakeCloud.AddSecurityGroupCalls).To(Equal(1)) securityGroups, err := fakeCloud.ListSecurityGroupsByTags(ctx, map[string]string{ util.LabelClusterName: clusterName, util.LabelResourceRole: util.ResourceRoleNodeSSH, @@ -245,6 +246,18 @@ var _ = Describe("StackitMachine Controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(securityGroups).To(HaveLen(1)) Expect(fakeCloud.SecurityGroupRemoteSources(securityGroups[0].ID)).To(ConsistOf("bastion-sg")) + Expect(fakeCloud.SecurityGroupRules(securityGroups[0].ID)).To(ConsistOf( + securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + RemoteSecurityGroupID: "bastion-sg", + }, + securitygroup.Rule{ + Direction: securitygroup.DirectionEgress, + }, + )) Expect(fakeCloud.ServerHasSecurityGroup(got.Status.InstanceID, securityGroups[0].ID)).To(BeTrue()) }) @@ -257,7 +270,7 @@ var _ = Describe("StackitMachine Controller", func() { stackitCluster.Status.Bastion.SecurityGroupID = "bastion-sg" Expect(k8sClient.Status().Update(ctx, stackitCluster)).To(Succeed()) - fakeCloud.FailNextEnsureNodeSSH = fmt.Errorf("node ssh access timeout: %w", cloud.ErrTransient) + fakeCloud.FailNextAddSecurityGroup = fmt.Errorf("node ssh access timeout: %w", cloud.ErrTransient) result, err := reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) diff --git a/controller/stackitmachine_infrastructure.go b/controller/stackitmachine_infrastructure.go index 920f016..ba1ae6a 100644 --- a/controller/stackitmachine_infrastructure.go +++ b/controller/stackitmachine_infrastructure.go @@ -29,7 +29,7 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" - bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/scope" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -91,6 +91,7 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS infrav1.MachineReadyCondition, ) } + machineScope.SetCloudClient(cloudClient) machineScope.SetConditions(metav1.ConditionTrue, "Available", "", infrav1.MachineCredentialsReadyCondition) server, created, err := r.ensureServer(ctx, cloudClient, machineScope, bootstrapData) @@ -160,7 +161,7 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS return ctrl.Result{RequeueAfter: 15 * time.Second}, nil } - if err := r.reconcileBastionNodeSSHAccess(ctx, cloudClient, machineScope, server); err != nil { + if err := r.reconcileBastionNodeSSHAccess(ctx, machineScope, server); err != nil { log.Error(err, "Failed to configure bastion SSH access for node", "machine", stackitMachine.Name) record.Warnf( stackitMachine, "FailedNodeSSHAccess", @@ -364,7 +365,6 @@ func (r *StackitMachineReconciler) ensureServer( func (r *StackitMachineReconciler) reconcileBastionNodeSSHAccess( ctx context.Context, - cloudClient cloud.Client, machineScope *scope.MachineScope, server *cloud.Server, ) error { @@ -377,13 +377,7 @@ func (r *StackitMachineReconciler) reconcileBastionNodeSSHAccess( if server == nil || server.ID == "" { return fmt.Errorf("%w: server ID is empty", cloud.ErrTransient) } - _, err := cloudClient.EnsureNodeSSHAccess(ctx, cloud.NodeSSHAccessInput{ - Name: machineScope.StackitCluster.Name + "-node-ssh", - ServerID: server.ID, - BastionSecurityGroupID: machineScope.StackitCluster.Status.Bastion.SecurityGroupID, - Tags: bastionservice.NodeSSHAccessTags(machineScope.StackitCluster), - }) - return err + return securitygroup.New(machineScope).Reconcile(ctx) } func (r *StackitMachineReconciler) getStackitCluster(ctx context.Context, cluster *clusterv1.Cluster) (*infrav1.StackitCluster, error) { diff --git a/scope/cluster_scope.go b/scope/cluster_scope.go index 0bb4c8b..af7c1bc 100644 --- a/scope/cluster_scope.go +++ b/scope/cluster_scope.go @@ -25,10 +25,13 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/util" iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) +const bastionSSHPort = 22 + // ClusterScope holds the per-reconcile state for a StackitCluster. type ClusterScope struct { Client client.Client @@ -91,10 +94,16 @@ func (s *ClusterScope) SetAPIServerEndpoint(endpoint clusterv1.APIEndpoint) { func (s *ClusterScope) SetBastionStatus(bastion *cloud.Bastion, cloudInitHash string) { s.StackitCluster.Status.Bastion.ServerID = bastion.ServerID - s.StackitCluster.Status.Bastion.SecurityGroupID = bastion.SecurityGroupID + if bastion.SecurityGroupID != "" { + s.StackitCluster.Status.Bastion.SecurityGroupID = bastion.SecurityGroupID + } s.StackitCluster.Status.Bastion.CloudInitHash = cloudInitHash } +func (s *ClusterScope) SetBastionSecurityGroupID(id string) { + s.StackitCluster.Status.Bastion.SecurityGroupID = id +} + func (s *ClusterScope) ClearBastionStatus() { s.StackitCluster.Status.Bastion = infrav1.StackitBastionStatus{} } @@ -160,12 +169,78 @@ func (s *ClusterScope) BastionStatus() infrav1.StackitBastionStatus { } func (s *ClusterScope) BastionTags() map[string]string { - tags := util.ClusterTags(s.StackitCluster.Name, s.StackitCluster.Namespace, s.StackitCluster.Spec.AdditionalLabels) + return bastionTags(s.StackitCluster) +} + +func bastionTags(stackitCluster *infrav1.StackitCluster) map[string]string { + tags := util.ClusterTags(stackitCluster.Name, stackitCluster.Namespace, stackitCluster.Spec.AdditionalLabels) tags[util.LabelResourceRole] = util.ResourceRoleBastion return tags } +func NodeSSHTags(stackitCluster *infrav1.StackitCluster) map[string]string { + tags := util.ClusterTags(stackitCluster.Name, stackitCluster.Namespace, stackitCluster.Spec.AdditionalLabels) + tags[util.LabelResourceRole] = util.ResourceRoleNodeSSH + return tags +} + func (s *ClusterScope) SetBastionPublicIP(id, ip string) { s.StackitCluster.Status.Bastion.PublicIPID = id s.StackitCluster.Status.Bastion.PublicIP = ip } + +func (s *ClusterScope) SecurityGroupSpecs() []securitygroup.SecurityGroupSpec { + rules := make([]securitygroup.Rule, 0, len(s.StackitCluster.Spec.Bastion.AllowedCIDRs)+1) + for _, cidr := range s.StackitCluster.Spec.Bastion.AllowedCIDRs { + if cidr == "" { + continue + } + rules = append(rules, securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: bastionSSHPort, + PortMax: bastionSSHPort, + IPRange: cidr, + }) + } + rules = append(rules, securitygroup.Rule{ + Direction: securitygroup.DirectionEgress, + }) + return []securitygroup.SecurityGroupSpec{ + { + ID: s.StackitCluster.Status.Bastion.SecurityGroupID, + Name: s.StackitCluster.Name + "-bastion-ssh", + Description: "Cluster API Provider STACKIT bastion SSH access", + Tags: s.BastionTags(), + Rules: rules, + SetID: s.SetBastionSecurityGroupID, + }, + nodeSSHSecurityGroupSpec(s.StackitCluster, ""), + } +} + +func nodeSSHSecurityGroupSpec(stackitCluster *infrav1.StackitCluster, remoteSecurityGroupID string, serverIDs ...string) securitygroup.SecurityGroupSpec { + ingressRule := securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: bastionSSHPort, + PortMax: bastionSSHPort, + RemoteSecurityGroupID: remoteSecurityGroupID, + } + if remoteSecurityGroupID == "" { + ingressRule.RemoteSecurityGroupTags = bastionTags(stackitCluster) + } + return securitygroup.SecurityGroupSpec{ + Name: stackitCluster.Name + "-node-ssh", + Description: "Cluster API Provider STACKIT node SSH access from bastion", + Tags: NodeSSHTags(stackitCluster), + ServerIDs: serverIDs, + ServerSelector: util.ClusterTags(stackitCluster.Name, stackitCluster.Namespace, nil), + Rules: []securitygroup.Rule{ + ingressRule, + { + Direction: securitygroup.DirectionEgress, + }, + }, + } +} diff --git a/scope/machine_scope.go b/scope/machine_scope.go index b736ce1..aea2b32 100644 --- a/scope/machine_scope.go +++ b/scope/machine_scope.go @@ -21,7 +21,9 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/util" + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) // MachineScope holds the per-reconcile state for a StackitMachine. @@ -31,6 +33,7 @@ type MachineScope struct { Machine *clusterv1.Machine StackitCluster *infrav1.StackitCluster StackitMachine *infrav1.StackitMachine + CloudClient cloud.Client patchHelper *patch.Helper } @@ -129,3 +132,35 @@ func (s *MachineScope) AvailabilityZone() string { return DefaultAvailabilityZone(s.StackitCluster) } + +func (s *MachineScope) SetCloudClient(cloudClient cloud.Client) { + s.CloudClient = cloudClient +} + +func (s *MachineScope) ProjectID() string { + if s.StackitCluster.Spec.ProjectID != "" { + return s.StackitCluster.Spec.ProjectID + } + return s.CloudClient.ProjectID() +} + +func (s *MachineScope) Region() string { + if s.StackitCluster.Spec.Region != "" { + return s.StackitCluster.Spec.Region + } + return s.CloudClient.Region() +} + +func (s *MachineScope) IaaSClient() *iaas.APIClient { + return s.CloudClient.IaaSClient() +} + +func (s *MachineScope) SecurityGroupSpecs() []securitygroup.SecurityGroupSpec { + return []securitygroup.SecurityGroupSpec{ + nodeSSHSecurityGroupSpec( + s.StackitCluster, + s.StackitCluster.Status.Bastion.SecurityGroupID, + s.StackitMachine.Status.InstanceID, + ), + } +} diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index d753b39..ebef540 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -35,6 +35,7 @@ import ( "github.com/stackitcloud/cluster-api-provider-stackit/cloud" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/test/utils" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -494,7 +495,7 @@ var _ = Describe("Manager", Ordered, func() { publicIPs, err := publicip.ListByTags(ctx, cloudClient.IaaSClient(), cloudClient.ProjectID(), cloudClient.Region(), stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(publicIPs).To(HaveLen(1)) - securityGroups, err := cloudClient.ListSecurityGroupsByTags(ctx, stackitE2EBastionTags(testID)) + securityGroups, err := securitygroup.NewClient(cloudClient).ListByTags(ctx, stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(securityGroups).To(HaveLen(1)) }, 10*time.Minute, 15*time.Second).Should(Succeed()) @@ -516,7 +517,7 @@ var _ = Describe("Manager", Ordered, func() { if cfg.BastionEnabled { By("verifying the node SSH security group exists") Eventually(func(g Gomega) { - securityGroups, err := cloudClient.ListSecurityGroupsByTags(ctx, stackitE2ENodeSSHTags(testID)) + securityGroups, err := securitygroup.NewClient(cloudClient).ListByTags(ctx, stackitE2ENodeSSHTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(securityGroups).To(HaveLen(1)) }, 10*time.Minute, 15*time.Second).Should(Succeed()) @@ -560,10 +561,11 @@ var _ = Describe("Manager", Ordered, func() { publicIPs, err := publicip.ListByTags(ctx, cloudClient.IaaSClient(), cloudClient.ProjectID(), cloudClient.Region(), stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(publicIPs).To(BeEmpty()) - securityGroups, err := cloudClient.ListSecurityGroupsByTags(ctx, stackitE2EBastionTags(testID)) + sgClient := securitygroup.NewClient(cloudClient) + securityGroups, err := sgClient.ListByTags(ctx, stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(securityGroups).To(BeEmpty()) - securityGroups, err = cloudClient.ListSecurityGroupsByTags(ctx, stackitE2ENodeSSHTags(testID)) + securityGroups, err = sgClient.ListByTags(ctx, stackitE2ENodeSSHTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(securityGroups).To(BeEmpty()) }, 20*time.Minute, 15*time.Second).Should(Succeed()) diff --git a/test/utils/utils.go b/test/utils/utils.go index a408630..9600e4b 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -35,10 +35,12 @@ const ( defaultKindCluster = "kind" ) +// TODO: unreachable func warnError(err error) { _, _ = fmt.Fprintf(GinkgoWriter, "warning: %v\n", err) } +// TODO: unreachable // Run executes the provided command within this context func Run(cmd *exec.Cmd) (string, error) { dir, _ := GetProjectDir() @@ -59,6 +61,7 @@ func Run(cmd *exec.Cmd) (string, error) { return string(output), nil } +// TODO: unreachable // UninstallCertManager uninstalls the cert manager func UninstallCertManager() { url := fmt.Sprintf(certmanagerURLTmpl, certmanagerVersion) @@ -81,6 +84,7 @@ func UninstallCertManager() { } } +// TODO: unreachable // InstallCertManager installs the cert manager bundle. func InstallCertManager() error { url := fmt.Sprintf(certmanagerURLTmpl, certmanagerVersion) @@ -100,6 +104,7 @@ func InstallCertManager() error { return err } +// TODO: unreachable // IsCertManagerCRDsInstalled checks if any Cert Manager CRDs are installed // by verifying the existence of key CRDs related to Cert Manager. func IsCertManagerCRDsInstalled() bool { @@ -133,6 +138,7 @@ func IsCertManagerCRDsInstalled() bool { return false } +// TODO: unreachable // LoadImageToKindClusterWithName loads a local docker image to the kind cluster func LoadImageToKindClusterWithName(name string) error { cluster := defaultKindCluster @@ -149,6 +155,7 @@ func LoadImageToKindClusterWithName(name string) error { return err } +// TODO: unreachable // GetNonEmptyLines converts given command output string into individual objects // according to line breakers, and ignores the empty elements in it. func GetNonEmptyLines(output string) []string { @@ -163,6 +170,7 @@ func GetNonEmptyLines(output string) []string { return res } +// TODO: unreachable // GetProjectDir will return the directory where the project is func GetProjectDir() (string, error) { wd, err := os.Getwd() @@ -173,6 +181,7 @@ func GetProjectDir() (string, error) { return wd, nil } +// TODO: unreachable // UncommentCode searches for target in the file and remove the comment prefix // of the target content. The target content may span multiple lines. func UncommentCode(filename, target, prefix string) error { From 254dd8b165ac48921d3e03d056ff3b45cacc14e4 Mon Sep 17 00:00:00 2001 From: Leonhard Schulze Date: Wed, 11 Nov 2026 11:37:29 +0100 Subject: [PATCH 6/6] refactor: fix merge --- cloud/fake/client.go | 76 --------------------- controller/stackitcluster_infrastructure.go | 8 +-- 2 files changed, 1 insertion(+), 83 deletions(-) diff --git a/cloud/fake/client.go b/cloud/fake/client.go index 1283943..5671191 100644 --- a/cloud/fake/client.go +++ b/cloud/fake/client.go @@ -489,82 +489,6 @@ func (c *Client) RemovePublicIPFromServer(_ context.Context, serverID, publicIPI return nil } -func (c *Client) CreatePublicIP(_ context.Context, tags map[string]string) (*cloud.PublicIP, error) { - c.mu.Lock() - defer c.mu.Unlock() - - if err := consume(&c.FailNextCreatePublicIP); err != nil { - return nil, err - } - id := c.genID() - publicIP := &cloud.PublicIP{ - ID: id, - IP: "203.0.113.22", - } - c.publicIPs[id] = &publicIPEntry{ - publicIP: publicIP, - tags: copyTags(tags), - } - out := *publicIP - return &out, nil -} - -func (c *Client) DeletePublicIP(_ context.Context, id string) error { - c.mu.Lock() - defer c.mu.Unlock() - - if err := consume(&c.FailNextDeletePublicIP); err != nil { - return err - } - if id == "" { - return fmt.Errorf("public IP ID is empty: %w", cloud.ErrInvalidInput) - } - if _, ok := c.publicIPs[id]; !ok { - return fmt.Errorf("public IP %q: %w", id, cloud.ErrNotFound) - } - delete(c.publicIPs, id) - return nil -} - -func (c *Client) AddPublicIPToServer(_ context.Context, serverID, publicIPID string) error { - c.mu.Lock() - defer c.mu.Unlock() - - if err := consume(&c.FailNextAddPublicIP); err != nil { - return err - } - if serverID == "" || publicIPID == "" { - return fmt.Errorf("server ID and public IP ID are required: %w", cloud.ErrInvalidInput) - } - if _, ok := c.servers[serverID]; !ok { - return fmt.Errorf("server %q: %w", serverID, cloud.ErrNotFound) - } - entry, ok := c.publicIPs[publicIPID] - if !ok { - return fmt.Errorf("public IP %q: %w", publicIPID, cloud.ErrNotFound) - } - entry.publicIP.NetworkInterfaceID = "fake-nic-" + serverID - return nil -} - -func (c *Client) RemovePublicIPFromServer(_ context.Context, serverID, publicIPID string) error { - c.mu.Lock() - defer c.mu.Unlock() - - if err := consume(&c.FailNextRemovePublicIP); err != nil { - return err - } - if serverID == "" || publicIPID == "" { - return fmt.Errorf("server ID and public IP ID are required: %w", cloud.ErrInvalidInput) - } - entry, ok := c.publicIPs[publicIPID] - if !ok { - return fmt.Errorf("public IP %q: %w", publicIPID, cloud.ErrNotFound) - } - entry.publicIP.NetworkInterfaceID = "" - return nil -} - func (c *Client) ListSecurityGroupsByTags(_ context.Context, tags map[string]string) ([]*cloud.SecurityGroup, error) { c.mu.Lock() defer c.mu.Unlock() diff --git a/controller/stackitcluster_infrastructure.go b/controller/stackitcluster_infrastructure.go index 2cf0a24..8bbaf7a 100644 --- a/controller/stackitcluster_infrastructure.go +++ b/controller/stackitcluster_infrastructure.go @@ -444,13 +444,7 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS "Failed to delete bastion public IP: %v", err) return ctrl.Result{}, err } - hadBastion := hasBastionStatus(stackitCluster.Status.Bastion) - if err := publicip.New(clusterScope).Delete(ctx); err != nil && !cloud.IsNotFound(err) { - log.Error(err, "Failed to delete bastion public IP", "cluster", stackitCluster.Name) - record.Warnf(stackitCluster, "FailedDeleteBastion", - "Failed to delete bastion public IP: %v", err) - return ctrl.Result{}, err - } + if err := cloudClient.DeleteBastion(ctx, bastionservice.Input(stackitCluster, nil), cloud.Bastion{ ServerID: stackitCluster.Status.Bastion.ServerID, SecurityGroupID: stackitCluster.Status.Bastion.SecurityGroupID,