From 5be7a32e6ab0c360ac93a1e0785ef02e06800374 Mon Sep 17 00:00:00 2001 From: Leonhard Schulze Date: Mon, 19 Oct 2026 18:02:57 +0200 Subject: [PATCH 1/4] 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/4] 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/4] 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/4] 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")