diff --git a/cloud/cleanup.go b/cloud/cleanup.go index 3d635e0..481f3de 100644 --- a/cloud/cleanup.go +++ b/cloud/cleanup.go @@ -17,6 +17,7 @@ import ( "strings" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" ) // CleanupByTags deletes provider-managed cloud resources matching tags without @@ -76,6 +77,18 @@ func CleanupByTags(ctx context.Context, client Client, tags map[string]string) e } } + sgClient := securitygroup.NewClient(client) + securityGroups, err := sgClient.ListByTags(ctx, tags) + if err != nil { + errs = append(errs, err.Error()) + } else { + for _, sg := range securityGroups { + if err := sgClient.DeleteByID(ctx, sg.ID); err != nil { + errs = append(errs, fmt.Sprintf("delete security group %q: %v", sg.ID, err)) + } + } + } + if len(errs) > 0 { return fmt.Errorf("%w: %s", ErrTransient, strings.Join(errs, "; ")) } diff --git a/cloud/cleanup_test.go b/cloud/cleanup_test.go index 40f2304..2fe2dc0 100644 --- a/cloud/cleanup_test.go +++ b/cloud/cleanup_test.go @@ -42,6 +42,12 @@ func TestCleanupByTagsDeletesMatchingCloudResources(t *testing.T) { if _, err := client.CreatePublicIP(ctx, otherTags); err != nil { t.Fatalf("CreatePublicIP() error = %v", err) } + if _, err := client.CreateSecurityGroup(ctx, "matched-sg", tags); err != nil { + t.Fatalf("CreateSecurityGroup() error = %v", err) + } + if _, err := client.CreateSecurityGroup(ctx, "other-sg", otherTags); err != nil { + t.Fatalf("CreateSecurityGroup() error = %v", err) + } if _, err := client.EnsureAPIServerLoadBalancer(ctx, cloud.LoadBalancerInput{ Name: "matched", Tags: tags, @@ -106,4 +112,18 @@ func TestCleanupByTagsDeletesMatchingCloudResources(t *testing.T) { if len(otherPublicIPs) != 1 { t.Fatalf("other public IP count = %d, want 1", len(otherPublicIPs)) } + matchedSecurityGroups, err := client.ListSecurityGroupsByTags(ctx, tags) + if err != nil { + t.Fatalf("ListSecurityGroupsByTags() error = %v", err) + } + if len(matchedSecurityGroups) != 0 { + t.Fatalf("matched security group count = %d, want 0", len(matchedSecurityGroups)) + } + otherSecurityGroups, err := client.ListSecurityGroupsByTags(ctx, otherTags) + if err != nil { + t.Fatalf("ListSecurityGroupsByTags(other) error = %v", err) + } + if len(otherSecurityGroups) != 1 { + t.Fatalf("other security group count = %d, want 1", len(otherSecurityGroups)) + } } diff --git a/cloud/client.go b/cloud/client.go index 7e5bd73..40b657a 100644 --- a/cloud/client.go +++ b/cloud/client.go @@ -55,12 +55,6 @@ type Client interface { DeleteBastion(ctx context.Context, input BastionInput, status Bastion) error - EnsureNodeSSHAccess(ctx context.Context, input NodeSSHAccessInput) (*SecurityGroup, error) - - DeleteNodeSSHAccess(ctx context.Context, tags map[string]string) error - - ListSecurityGroupsByTags(ctx context.Context, tags map[string]string) ([]*SecurityGroup, error) - EnsureAPIServerLoadBalancer(ctx context.Context, input LoadBalancerInput) (*LoadBalancer, error) ListAPIServerLoadBalancersByTags(ctx context.Context, tags map[string]string) ([]*LoadBalancer, error) diff --git a/cloud/fake/client.go b/cloud/fake/client.go index 0f5c589..5671191 100644 --- a/cloud/fake/client.go +++ b/cloud/fake/client.go @@ -24,6 +24,7 @@ import ( "sync" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/stackit-sdk-go/core/config" iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) @@ -48,25 +49,24 @@ type Client struct { // failure injection: if non-nil, the next call returns this error and the // field is cleared. - FailNextCreateServer error - FailNextDeleteServer error - FailNextGetServer error - FailNextFindServer error - FailNextEnsureLB error - FailNextDeleteLB error - FailNextSetTargets error - FailNextGetNetwork error - FailNextEnsureBastion error - FailNextDeleteBastion error - FailNextEnsureNodeSSH error - FailNextDeleteNodeSSH error - FailNextListAZ error - FailNextGetPublicIP error - FailNextListPublicIPs error - FailNextCreatePublicIP error - FailNextDeletePublicIP error - FailNextAddPublicIP error - FailNextRemovePublicIP error + FailNextCreateServer error + FailNextDeleteServer error + FailNextGetServer error + FailNextFindServer error + FailNextEnsureLB error + FailNextDeleteLB error + FailNextSetTargets error + FailNextGetNetwork error + FailNextEnsureBastion error + FailNextDeleteBastion error + FailNextAddSecurityGroup error + FailNextListAZ error + FailNextGetPublicIP error + FailNextListPublicIPs error + FailNextCreatePublicIP error + FailNextDeletePublicIP error + FailNextAddPublicIP error + FailNextRemovePublicIP error // AvailabilityZones is returned by ListAvailabilityZones if non-empty. // Defaults to eu01-1, eu01-2, eu01-3. @@ -86,9 +86,9 @@ type Client struct { // CreateServerCalls counts successful CreateServer calls (for idempotency // assertions). - CreateServerCalls int - EnsureBastionCalls int - EnsureNodeSSHCalls int + CreateServerCalls int + EnsureBastionCalls int + AddSecurityGroupCalls int } type serverEntry struct { @@ -109,11 +109,18 @@ type publicIPEntry struct { tags map[string]string } +type securityGroupRuleEntry struct { + id string + securitygroup.Rule +} + type securityGroupEntry struct { securityGroup *cloud.SecurityGroup + description string tags map[string]string allowedCIDRs []string remoteSecurityGroups []string + rules map[string]*securityGroupRuleEntry } // New returns a Client preconfigured with the given networks. @@ -295,29 +302,27 @@ func (c *Client) EnsureBastion(_ context.Context, input cloud.BastionInput) (*cl if err := consume(&c.FailNextEnsureBastion); err != nil { return nil, err } + securityGroupID := input.SecurityGroupID + if securityGroupID == "" { + securityGroupID = idOfSecurityGroup(c.securityGroupByTags(input.Tags)) + } for _, serverEntry := range c.servers { if len(input.Tags) > 0 && mapContains(serverEntry.tags, input.Tags) { - securityGroup := c.securityGroupByTags(input.Tags) + if securityGroupID != "" { + if serverEntry.securityGroups == nil { + serverEntry.securityGroups = map[string]struct{}{} + } + serverEntry.securityGroups[securityGroupID] = struct{}{} + } return &cloud.Bastion{ ServerID: serverEntry.server.ID, ServerState: serverEntry.server.State, ServerPowerStatus: serverEntry.server.PowerStatus, - SecurityGroupID: idOfSecurityGroup(securityGroup), + SecurityGroupID: securityGroupID, }, nil } } - securityGroupID := c.genID() - securityGroup := &cloud.SecurityGroup{ - ID: securityGroupID, - Name: input.Name + "-ssh", - } - c.securityGroups[securityGroupID] = &securityGroupEntry{ - securityGroup: securityGroup, - tags: copyTags(input.Tags), - allowedCIDRs: append([]string(nil), input.AllowedCIDRs...), - } - serverID := c.genID() c.LastEnsureBastionInput = input zone := input.AvailabilityZone @@ -333,10 +338,14 @@ func (c *Client) EnsureBastion(_ context.Context, input cloud.BastionInput) (*cl {Type: "InternalIP", Address: "10.0.0.20"}, }, } + var sgSet map[string]struct{} + if securityGroupID != "" { + sgSet = securityGroupSet([]string{securityGroupID}) + } c.servers[serverID] = &serverEntry{ server: server, tags: copyTags(input.Tags), - securityGroups: securityGroupSet([]string{securityGroupID}), + securityGroups: sgSet, userData: append([]byte(nil), input.CloudInit...), } @@ -358,74 +367,11 @@ func (c *Client) DeleteBastion(_ context.Context, input cloud.BastionInput, stat if status.ServerID != "" { delete(c.servers, status.ServerID) } - if status.SecurityGroupID != "" { - delete(c.securityGroups, status.SecurityGroupID) - } for id, entry := range c.servers { - if mapContains(entry.tags, input.Tags) { + if len(input.Tags) > 0 && mapContains(entry.tags, input.Tags) { delete(c.servers, id) } } - for id, entry := range c.securityGroups { - if mapContains(entry.tags, input.Tags) { - delete(c.securityGroups, id) - } - } - return nil -} - -func (c *Client) EnsureNodeSSHAccess(_ context.Context, input cloud.NodeSSHAccessInput) (*cloud.SecurityGroup, error) { - c.mu.Lock() - defer c.mu.Unlock() - - if err := consume(&c.FailNextEnsureNodeSSH); err != nil { - return nil, err - } - serverEntry, ok := c.servers[input.ServerID] - if !ok { - return nil, fmt.Errorf("server %q: %w", input.ServerID, cloud.ErrNotFound) - } - securityGroup := c.securityGroupByTags(input.Tags) - if securityGroup == nil { - securityGroup = &cloud.SecurityGroup{ - ID: c.genID(), - Name: input.Name, - } - c.securityGroups[securityGroup.ID] = &securityGroupEntry{ - securityGroup: securityGroup, - tags: copyTags(input.Tags), - remoteSecurityGroups: []string{input.BastionSecurityGroupID}, - } - } else { - entry := c.securityGroups[securityGroup.ID] - if !containsString(entry.remoteSecurityGroups, input.BastionSecurityGroupID) { - entry.remoteSecurityGroups = append(entry.remoteSecurityGroups, input.BastionSecurityGroupID) - } - } - if serverEntry.securityGroups == nil { - serverEntry.securityGroups = map[string]struct{}{} - } - serverEntry.securityGroups[securityGroup.ID] = struct{}{} - c.EnsureNodeSSHCalls++ - out := *securityGroup - return &out, nil -} - -func (c *Client) DeleteNodeSSHAccess(_ context.Context, tags map[string]string) error { - c.mu.Lock() - defer c.mu.Unlock() - - if err := consume(&c.FailNextDeleteNodeSSH); err != nil { - return err - } - for id, entry := range c.securityGroups { - if mapContains(entry.tags, tags) { - delete(c.securityGroups, id) - for _, server := range c.servers { - delete(server.securityGroups, id) - } - } - } return nil } @@ -560,6 +506,18 @@ func (c *Client) ListSecurityGroupsByTags(_ context.Context, tags map[string]str return securityGroups, nil } +func (c *Client) CreateSecurityGroup(_ context.Context, name string, tags map[string]string) (*cloud.SecurityGroup, error) { + c.mu.Lock() + defer c.mu.Unlock() + + entry, err := c.createSecurityGroupEntry(name, "", tags) + if err != nil { + return nil, err + } + out := *entry.securityGroup + return &out, nil +} + func (c *Client) EnsureAPIServerLoadBalancer( _ context.Context, input cloud.LoadBalancerInput, @@ -736,6 +694,21 @@ func (c *Client) SecurityGroupRemoteSources(securityGroupID string) []string { return append([]string(nil), entry.remoteSecurityGroups...) } +// SecurityGroupRules returns the rules currently stored on a fake security group. +func (c *Client) SecurityGroupRules(securityGroupID string) []securitygroup.Rule { + c.mu.Lock() + defer c.mu.Unlock() + entry, ok := c.securityGroups[securityGroupID] + if !ok { + return nil + } + rules := make([]securitygroup.Rule, 0, len(entry.rules)) + for _, r := range entry.rules { + rules = append(rules, r.Rule) + } + return rules +} + // LoadBalancerTargetCount returns the number of targets in one load balancer // target pool (test helper). func (c *Client) LoadBalancerTargetCount(id string) int { @@ -850,6 +823,17 @@ func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { rec := httptest.NewRecorder() rec.Header().Set("Content-Type", "application/json") + + if c.handlePublicIPRequest(rec, r) || c.handleSecurityGroupRequest(rec, r) { + return rec.Result(), nil + } + + rec.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(rec).Encode(map[string]any{"message": "not found"}) + return rec.Result(), nil +} + +func (c *Client) handlePublicIPRequest(rec *httptest.ResponseRecorder, r *http.Request) bool { path := r.URL.Path switch { @@ -866,11 +850,8 @@ func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { default: err = cloud.ErrInvalidInput } - if err != nil { - writeFakeError(rec, err) - } else { - rec.WriteHeader(http.StatusNoContent) - } + writeFakeStatus(rec, err) + return true case r.Method == http.MethodPost && strings.HasSuffix(path, "/public-ips"): var body struct { @@ -885,6 +866,7 @@ func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { } else { _ = json.NewEncoder(rec).Encode(fakePublicIPMap(created, body.Labels)) } + return true case r.Method == http.MethodGet && strings.HasSuffix(path, "/public-ips"): tags := parseLabelSelector(r.URL.Query().Get("label_selector")) @@ -894,6 +876,7 @@ func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { } else { _ = json.NewEncoder(rec).Encode(map[string]any{"items": items}) } + return true case r.Method == http.MethodGet && strings.Contains(path, "/public-ips/"): id := path[strings.LastIndex(path, "/public-ips/")+len("/public-ips/"):] @@ -903,21 +886,151 @@ func (c *Client) roundTripIaaS(r *http.Request) (*http.Response, error) { } else { _ = json.NewEncoder(rec).Encode(fakePublicIPMap(got, nil)) } + return true case r.Method == http.MethodDelete && strings.Contains(path, "/public-ips/"): id := path[strings.LastIndex(path, "/public-ips/")+len("/public-ips/"):] - if err := c.DeletePublicIP(r.Context(), id); err != nil { + writeFakeStatus(rec, c.DeletePublicIP(r.Context(), id)) + return true + + default: + return false + } +} + +func (c *Client) handleSecurityGroupRequest(rec *httptest.ResponseRecorder, r *http.Request) bool { + path := r.URL.Path + + switch { + case strings.Contains(path, "/servers/") && strings.Contains(path, "/security-groups/"): + parts := strings.Split(path, "/servers/") + sub := strings.Split(parts[1], "/security-groups/") + serverID, securityGroupID := sub[0], sub[1] + var err error + switch r.Method { + case http.MethodPut: + err = c.addSecurityGroupToServer(serverID, securityGroupID) + case http.MethodDelete: + err = c.removeSecurityGroupFromServer(serverID, securityGroupID) + default: + err = cloud.ErrInvalidInput + } + writeFakeStatus(rec, err) + return true + + case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): + tags := parseLabelSelector(r.URL.Query().Get("label_selector")) + items, err := c.listServerItems(tags) + if err != nil { writeFakeError(rec, err) } else { - rec.WriteHeader(http.StatusNoContent) + _ = json.NewEncoder(rec).Encode(map[string]any{"items": items}) } + return true + + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups"): + var body struct { + Name string `json:"name"` + Description string `json:"description"` + Labels map[string]string `json:"labels"` + } + if r.Body != nil { + _ = json.NewDecoder(r.Body).Decode(&body) + } + created, err := c.createSecurityGroup(body.Name, body.Description, body.Labels) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(created) + } + return true + + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + tags := parseLabelSelector(r.URL.Query().Get("label_selector")) + items, err := c.listSecurityGroupItems(tags) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(map[string]any{"items": items}) + } + return true + + case strings.Contains(path, "/security-groups/") && strings.HasSuffix(path, "/rules") && r.Method == http.MethodGet: + sub := path[strings.Index(path, "/security-groups/")+len("/security-groups/"):] + sgID := strings.TrimSuffix(sub, "/rules") + items, err := c.listSecurityGroupRuleItems(sgID) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(map[string]any{"items": items}) + } + return true + + case strings.Contains(path, "/security-groups/") && strings.HasSuffix(path, "/rules") && r.Method == http.MethodPost: + c.handleCreateSecurityGroupRule(rec, r) + return true + + case strings.Contains(path, "/security-groups/") && strings.Contains(path, "/rules/") && r.Method == http.MethodDelete: + sub := path[strings.Index(path, "/security-groups/")+len("/security-groups/"):] + parts := strings.SplitN(sub, "/rules/", 2) + writeFakeStatus(rec, c.deleteSecurityGroupRule(parts[0], parts[1])) + return true + + case r.Method == http.MethodDelete && strings.Contains(path, "/security-groups/"): + id := path[strings.LastIndex(path, "/security-groups/")+len("/security-groups/"):] + writeFakeStatus(rec, c.deleteSecurityGroup(id)) + return true default: - rec.WriteHeader(http.StatusNotFound) - _ = json.NewEncoder(rec).Encode(map[string]any{"message": "not found"}) + return false } +} - return rec.Result(), nil +func (c *Client) handleCreateSecurityGroupRule(rec *httptest.ResponseRecorder, r *http.Request) { + path := r.URL.Path + sub := path[strings.Index(path, "/security-groups/")+len("/security-groups/"):] + sgID := strings.TrimSuffix(sub, "/rules") + var body struct { + Direction string `json:"direction"` + Protocol string `json:"protocol"` + PortRange *struct { + Min int64 `json:"min"` + Max int64 `json:"max"` + } `json:"portRange"` + IPRange string `json:"ipRange"` + RemoteSecurityGroupID string `json:"remoteSecurityGroupId"` + } + if r.Body != nil { + _ = json.NewDecoder(r.Body).Decode(&body) + } + var portMin, portMax int64 + if body.PortRange != nil { + portMin = body.PortRange.Min + portMax = body.PortRange.Max + } + created, err := c.createSecurityGroupRule(sgID, securityGroupRuleEntry{ + Rule: securitygroup.Rule{ + Direction: securitygroup.Direction(body.Direction), + Protocol: securitygroup.Protocol(body.Protocol), + PortMin: portMin, + PortMax: portMax, + IPRange: body.IPRange, + RemoteSecurityGroupID: body.RemoteSecurityGroupID, + }, + }) + if err != nil { + writeFakeError(rec, err) + } else { + _ = json.NewEncoder(rec).Encode(created) + } +} + +func writeFakeStatus(rec *httptest.ResponseRecorder, err error) { + if err != nil { + writeFakeError(rec, err) + } else { + rec.WriteHeader(http.StatusNoContent) + } } func (c *Client) listPublicIPItems(tags map[string]string) ([]map[string]any, error) { @@ -957,6 +1070,226 @@ func fakePublicIPMap(p *cloud.PublicIP, tags map[string]string) map[string]any { return m } +func (c *Client) addSecurityGroupToServer(serverID, securityGroupID string) error { + c.mu.Lock() + defer c.mu.Unlock() + + if err := consume(&c.FailNextAddSecurityGroup); err != nil { + return err + } + serverEntry, ok := c.servers[serverID] + if !ok { + return fmt.Errorf("server %q: %w", serverID, cloud.ErrNotFound) + } + if _, ok := c.securityGroups[securityGroupID]; !ok { + return fmt.Errorf("security group %q: %w", securityGroupID, cloud.ErrNotFound) + } + if serverEntry.securityGroups == nil { + serverEntry.securityGroups = map[string]struct{}{} + } + serverEntry.securityGroups[securityGroupID] = struct{}{} + c.AddSecurityGroupCalls++ + return nil +} + +func (c *Client) listServerItems(tags map[string]string) ([]map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + items := make([]map[string]any, 0, len(c.servers)) + for _, entry := range c.servers { + if len(tags) > 0 && !mapContains(entry.tags, tags) { + continue + } + m := map[string]any{ + "id": entry.server.ID, + "name": entry.server.Name, + "machineType": "c2i.1", + "status": entry.server.State, + } + if len(entry.tags) > 0 { + labels := make(map[string]any, len(entry.tags)) + for k, v := range entry.tags { + labels[k] = v + } + m["labels"] = labels + } + items = append(items, m) + } + return items, nil +} + +func (c *Client) removeSecurityGroupFromServer(serverID, securityGroupID string) error { + c.mu.Lock() + defer c.mu.Unlock() + + serverEntry, ok := c.servers[serverID] + if !ok { + return fmt.Errorf("server %q: %w", serverID, cloud.ErrNotFound) + } + delete(serverEntry.securityGroups, securityGroupID) + return nil +} + +func (c *Client) createSecurityGroup(name, description string, tags map[string]string) (map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + entry, err := c.createSecurityGroupEntry(name, description, tags) + if err != nil { + return nil, err + } + return fakeSecurityGroupMap(entry), nil +} + +func (c *Client) createSecurityGroupEntry(name, description string, tags map[string]string) (*securityGroupEntry, error) { + if name == "" { + return nil, fmt.Errorf("security group name is empty: %w", cloud.ErrInvalidInput) + } + id := c.genID() + sg := &cloud.SecurityGroup{ + ID: id, + Name: name, + } + entry := &securityGroupEntry{ + securityGroup: sg, + description: description, + tags: copyTags(tags), + rules: make(map[string]*securityGroupRuleEntry), + } + c.securityGroups[id] = entry + return entry, nil +} + +func (c *Client) listSecurityGroupItems(tags map[string]string) ([]map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + if len(tags) == 0 { + return nil, fmt.Errorf("empty tags: %w", cloud.ErrInvalidInput) + } + items := make([]map[string]any, 0, len(c.securityGroups)) + for _, entry := range c.securityGroups { + if mapContains(entry.tags, tags) { + items = append(items, fakeSecurityGroupMap(entry)) + } + } + return items, nil +} + +func (c *Client) listSecurityGroupRuleItems(sgID string) ([]map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + entry, ok := c.securityGroups[sgID] + if !ok { + return nil, fmt.Errorf("security group %q: %w", sgID, cloud.ErrNotFound) + } + items := make([]map[string]any, 0, len(entry.rules)) + for _, rule := range entry.rules { + items = append(items, fakeSecurityGroupRuleMap(rule)) + } + return items, nil +} + +func (c *Client) createSecurityGroupRule(sgID string, rule securityGroupRuleEntry) (map[string]any, error) { + c.mu.Lock() + defer c.mu.Unlock() + + entry, ok := c.securityGroups[sgID] + if !ok { + return nil, fmt.Errorf("security group %q: %w", sgID, cloud.ErrNotFound) + } + if entry.rules == nil { + entry.rules = make(map[string]*securityGroupRuleEntry) + } + rule.id = c.genID() + stored := rule + entry.rules[rule.id] = &stored + if rule.IPRange != "" && !containsString(entry.allowedCIDRs, rule.IPRange) { + entry.allowedCIDRs = append(entry.allowedCIDRs, rule.IPRange) + } + if rule.RemoteSecurityGroupID != "" && !containsString(entry.remoteSecurityGroups, rule.RemoteSecurityGroupID) { + entry.remoteSecurityGroups = append(entry.remoteSecurityGroups, rule.RemoteSecurityGroupID) + } + return fakeSecurityGroupRuleMap(&stored), nil +} + +func (c *Client) deleteSecurityGroupRule(sgID, ruleID string) error { + c.mu.Lock() + defer c.mu.Unlock() + + entry, ok := c.securityGroups[sgID] + if !ok { + return fmt.Errorf("security group %q: %w", sgID, cloud.ErrNotFound) + } + if entry.rules == nil { + return fmt.Errorf("rule %q: %w", ruleID, cloud.ErrNotFound) + } + if _, ok := entry.rules[ruleID]; !ok { + return fmt.Errorf("rule %q: %w", ruleID, cloud.ErrNotFound) + } + delete(entry.rules, ruleID) + return nil +} + +func (c *Client) deleteSecurityGroup(id string) error { + c.mu.Lock() + defer c.mu.Unlock() + + if id == "" { + return fmt.Errorf("security group ID is empty: %w", cloud.ErrInvalidInput) + } + if _, ok := c.securityGroups[id]; !ok { + return fmt.Errorf("security group %q: %w", id, cloud.ErrNotFound) + } + for serverID, server := range c.servers { + if _, attached := server.securityGroups[id]; attached { + return fmt.Errorf("security group %q is still attached to server %q: %w", id, serverID, cloud.ErrConflict) + } + } + delete(c.securityGroups, id) + return nil +} + +func fakeSecurityGroupMap(entry *securityGroupEntry) map[string]any { + m := map[string]any{ + "id": entry.securityGroup.ID, + "name": entry.securityGroup.Name, + } + if entry.description != "" { + m["description"] = entry.description + } + if len(entry.tags) > 0 { + labels := make(map[string]any, len(entry.tags)) + for k, v := range entry.tags { + labels[k] = v + } + m["labels"] = labels + } + return m +} + +func fakeSecurityGroupRuleMap(rule *securityGroupRuleEntry) map[string]any { + m := map[string]any{ + "id": rule.id, + "direction": string(rule.Direction), + } + if rule.Protocol != "" { + m["protocol"] = map[string]any{"name": string(rule.Protocol)} + } + if rule.PortMin > 0 || rule.PortMax > 0 { + m["portRange"] = map[string]any{"min": rule.PortMin, "max": rule.PortMax} + } + if rule.IPRange != "" { + m["ipRange"] = rule.IPRange + } + if rule.RemoteSecurityGroupID != "" { + m["remoteSecurityGroupId"] = rule.RemoteSecurityGroupID + } + return m +} + func parseLabelSelector(selector string) map[string]string { if selector == "" { return nil diff --git a/cloud/sdk_client.go b/cloud/sdk_client.go index 531b7ec..4777106 100644 --- a/cloud/sdk_client.go +++ b/cloud/sdk_client.go @@ -19,6 +19,7 @@ import ( "slices" "strings" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" "github.com/stackitcloud/stackit-sdk-go/core/config" iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" @@ -35,8 +36,6 @@ const ( apiserverListenerName = "apiserver" bootstrapTargetName = "capi-bootstrap-placeholder" bootstrapTargetIP = "10.0.0.1" - sshPort = 22 - tcpProtocol = "tcp" ) // SDKClient is the STACKIT SDK-backed implementation of Client. @@ -259,12 +258,14 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas return nil, fmt.Errorf("%w: bastion tags are required", ErrInvalidInput) } - securityGroup, err := c.ensureBastionSecurityGroup(ctx, input) - if err != nil { - return nil, err - } - if err := c.ensureBastionSecurityGroupRules(ctx, securityGroup.ID, input.AllowedCIDRs); err != nil { - return nil, err + sgClient := securitygroup.NewClient(c) + securityGroupID := input.SecurityGroupID + if securityGroupID == "" { + securityGroup, err := sgClient.FindByTags(ctx, input.Tags) + if err != nil { + return nil, err + } + securityGroupID = securityGroup.ID } server, err := c.CreateServer(ctx, CreateServerInput{ @@ -276,7 +277,7 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas AvailabilityZone: input.AvailabilityZone, SSHKeyName: input.SSHKeyName, NetworkID: input.NetworkID, - SecurityGroups: []string{securityGroup.ID}, + SecurityGroups: []string{securityGroupID}, UserData: input.CloudInit, Tags: input.Tags, RootVolume: input.RootVolume, @@ -290,7 +291,7 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas // by an interrupted DeleteBastion). Kept deliberately, and idempotent: the // duplicate/no-port-yet responses are tolerated so they can no longer abort // the reconcile before the public IP below is assigned. - if err := c.addSecurityGroupToServer(ctx, server.ID, securityGroup.ID); err != nil { + if err := sgClient.AttachToServer(ctx, server.ID, securityGroupID); err != nil { return nil, err } @@ -298,7 +299,7 @@ func (c *SDKClient) EnsureBastion(ctx context.Context, input BastionInput) (*Bas ServerID: server.ID, ServerState: server.State, ServerPowerStatus: server.PowerStatus, - SecurityGroupID: securityGroup.ID, + SecurityGroupID: securityGroupID, }, nil } @@ -312,10 +313,17 @@ func (c *SDKClient) DeleteBastion(ctx context.Context, input BastionInput, statu return err } } + if serverID == "" { + return nil + } + sgClient := securitygroup.NewClient(c) securityGroupID := status.SecurityGroupID if securityGroupID == "" { - securityGroup, err := c.findSecurityGroupByTags(ctx, input.Tags) + securityGroupID = input.SecurityGroupID + } + if securityGroupID == "" && len(input.Tags) > 0 { + securityGroup, err := sgClient.FindByTags(ctx, input.Tags) if err == nil { securityGroupID = securityGroup.ID } else if !IsNotFound(err) { @@ -323,117 +331,17 @@ func (c *SDKClient) DeleteBastion(ctx context.Context, input BastionInput, statu } } - if serverID != "" && securityGroupID != "" { - if err := c.iaasClient.DefaultAPI. - RemoveSecurityGroupFromServer(ctx, c.projectID, c.region, serverID, securityGroupID). - Execute(); err != nil { - err := shared.ClassifySDKError("remove security group from server", err) - if !IsNotFound(err) && !IsConflict(err) { - return err - } - } - } - if serverID != "" { - if err := c.DeleteServer(ctx, serverID); err != nil && !IsNotFound(err) { - return err - } - } if securityGroupID != "" { - if err := c.deleteSecurityGroupRules(ctx, securityGroupID); err != nil { + if err := sgClient.DetachFromServer(ctx, serverID, securityGroupID); err != nil { return err } - if err := c.iaasClient.DefaultAPI. - DeleteSecurityGroup(ctx, c.projectID, c.region, securityGroupID). - Execute(); err != nil { - err := shared.ClassifySDKError("delete security group", err) - if !IsNotFound(err) { - return err - } - } - } - return nil -} - -func (c *SDKClient) EnsureNodeSSHAccess(ctx context.Context, input NodeSSHAccessInput) (*SecurityGroup, error) { - if input.Name == "" || input.ServerID == "" || input.BastionSecurityGroupID == "" { - return nil, fmt.Errorf( - "%w: node SSH access name, server ID, and bastion security group ID are required", - ErrInvalidInput, - ) - } - if len(input.Tags) == 0 { - return nil, fmt.Errorf("%w: node SSH access tags are required", ErrInvalidInput) - } - - securityGroup, err := c.ensureNodeSSHSecurityGroup(ctx, input) - if err != nil { - return nil, err - } - if err := c.ensureNodeSSHSecurityGroupRule(ctx, securityGroup.ID, input.BastionSecurityGroupID); err != nil { - return nil, err - } - if err := c.addSecurityGroupToServer(ctx, input.ServerID, securityGroup.ID); err != nil { - return nil, err - } - return securityGroup, nil -} - -func (c *SDKClient) DeleteNodeSSHAccess(ctx context.Context, tags map[string]string) error { - securityGroup, err := c.findSecurityGroupByTags(ctx, tags) - if IsNotFound(err) { - return nil - } - if err != nil { - return err - } - servers, err := c.ListServersByTags(ctx, clusterServerSelectorFromTags(tags)) - if err != nil { - return err - } - for _, server := range servers { - if err := c.iaasClient.DefaultAPI. - RemoveSecurityGroupFromServer(ctx, c.projectID, c.region, server.ID, securityGroup.ID). - Execute(); err != nil { - err := shared.ClassifySDKError("remove security group from server", err) - if !IsNotFound(err) && !IsConflict(err) { - return err - } - } } - if err := c.deleteSecurityGroupRules(ctx, securityGroup.ID); err != nil { + if err := c.DeleteServer(ctx, serverID); err != nil && !IsNotFound(err) { return err } - if err := c.iaasClient.DefaultAPI. - DeleteSecurityGroup(ctx, c.projectID, c.region, securityGroup.ID). - Execute(); err != nil { - err := shared.ClassifySDKError("delete security group", err) - if !IsNotFound(err) { - return err - } - } return nil } -func (c *SDKClient) ListSecurityGroupsByTags(ctx context.Context, tags map[string]string) ([]*SecurityGroup, error) { - if len(tags) == 0 { - return nil, fmt.Errorf("%w: empty tag selector", ErrInvalidInput) - } - resp, err := c.iaasClient.DefaultAPI.ListSecurityGroups(ctx, c.projectID, c.region). - LabelSelector(shared.LabelSelector(tags)). - Execute() - if err != nil { - return nil, shared.ClassifySDKError("list security groups", err) - } - matched := []*SecurityGroup{} - for _, securityGroup := range resp.GetItems() { - if shared.LabelsContainTags(securityGroup.GetLabels(), tags) { - securityGroup := securityGroup - matched = append(matched, securityGroupFromSDK(&securityGroup)) - } - } - return matched, nil -} - func (c *SDKClient) EnsureAPIServerLoadBalancer(ctx context.Context, input LoadBalancerInput) (*LoadBalancer, error) { if existing, err := c.findLoadBalancerByTags(ctx, input.Tags); err == nil { return existing, nil @@ -604,229 +512,6 @@ func (c *SDKClient) findLoadBalancerByTags(ctx context.Context, tags map[string] return matched[0], nil } -func (c *SDKClient) ensureBastionSecurityGroup(ctx context.Context, input BastionInput) (*SecurityGroup, error) { - if existing, err := c.findSecurityGroupByTags(ctx, input.Tags); err == nil { - return existing, nil - } else if !IsNotFound(err) { - return nil, err - } - payload := iaas.NewCreateSecurityGroupPayload(input.Name + "-ssh") - payload.SetDescription("Cluster API Provider STACKIT bastion SSH access") - payload.SetLabels(shared.TagsToSDKLabels(input.Tags)) - securityGroup, err := c.iaasClient.DefaultAPI.CreateSecurityGroup(ctx, c.projectID, c.region). - CreateSecurityGroupPayload(*payload). - Execute() - if err != nil { - return nil, shared.ClassifySDKError("create security group", err) - } - return securityGroupFromSDK(securityGroup), nil -} - -func (c *SDKClient) ensureNodeSSHSecurityGroup(ctx context.Context, input NodeSSHAccessInput) (*SecurityGroup, error) { - if existing, err := c.findSecurityGroupByTags(ctx, input.Tags); err == nil { - return existing, nil - } else if !IsNotFound(err) { - return nil, err - } - payload := iaas.NewCreateSecurityGroupPayload(input.Name) - payload.SetDescription("Cluster API Provider STACKIT node SSH access from bastion") - payload.SetLabels(shared.TagsToSDKLabels(input.Tags)) - securityGroup, err := c.iaasClient.DefaultAPI.CreateSecurityGroup(ctx, c.projectID, c.region). - CreateSecurityGroupPayload(*payload). - Execute() - if err != nil { - return nil, shared.ClassifySDKError("create security group", err) - } - return securityGroupFromSDK(securityGroup), nil -} - -func (c *SDKClient) ensureBastionSecurityGroupRules(ctx context.Context, securityGroupID string, cidrs []string) error { - if securityGroupID == "" { - return fmt.Errorf("%w: security group ID is required", ErrInvalidInput) - } - resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, securityGroupID).Execute() - if err != nil { - return shared.ClassifySDKError("list security group rules", err) - } - existingRules := resp.GetItems() - desired := make(map[string]struct{}, len(cidrs)) - for _, cidr := range cidrs { - if cidr == "" { - return fmt.Errorf("%w: empty bastion allowed CIDR", ErrInvalidInput) - } - if _, seen := desired[cidr]; seen { - // A CIDR listed twice would otherwise be created twice: - // existingRules is a snapshot from before this loop, so the second - // pass does not see the rule the first pass just created, and the - // duplicate create fails the whole bastion reconcile. - continue - } - desired[cidr] = struct{}{} - if hasSSHRule(existingRules, cidr) { - continue - } - protocol := iaas.StringAsCreateProtocol(new(tcpProtocol)) - rule := iaas.NewCreateSecurityGroupRulePayload("ingress") - rule.SetIpRange(cidr) - rule.SetPortRange(*iaas.NewPortRange(sshPort, sshPort)) - rule.SetProtocol(protocol) - if _, err := c.iaasClient.DefaultAPI.CreateSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID). - CreateSecurityGroupRulePayload(*rule). - Execute(); err != nil { - return shared.ClassifySDKError("create security group rule", err) - } - } - - // Revoke SSH rules for CIDRs that are no longer desired. Without this the - // rule set only ever grows, so narrowing allowedCIDRs would not actually - // take access away from the previously allowed range. - for _, rule := range existingRules { - if !isSSHRule(rule) { - continue - } - if _, keep := desired[rule.GetIpRange()]; keep { - continue - } - ruleID := rule.GetId() - if ruleID == "" { - continue - } - if err := c.iaasClient.DefaultAPI. - DeleteSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID, ruleID). - Execute(); err != nil { - err := shared.ClassifySDKError("delete security group rule", err) - if !IsNotFound(err) { - return err - } - } - } - return nil -} - -func (c *SDKClient) ensureNodeSSHSecurityGroupRule( - ctx context.Context, - nodeSecurityGroupID, bastionSecurityGroupID string, -) error { - if nodeSecurityGroupID == "" || bastionSecurityGroupID == "" { - return fmt.Errorf("%w: node and bastion security group IDs are required", ErrInvalidInput) - } - resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, nodeSecurityGroupID).Execute() - if err != nil { - return shared.ClassifySDKError("list security group rules", err) - } - if hasRemoteSecurityGroupSSHRule(resp.GetItems(), bastionSecurityGroupID) { - return nil - } - protocol := iaas.StringAsCreateProtocol(new(tcpProtocol)) - rule := iaas.NewCreateSecurityGroupRulePayload("ingress") - rule.SetRemoteSecurityGroupId(bastionSecurityGroupID) - rule.SetPortRange(*iaas.NewPortRange(sshPort, sshPort)) - rule.SetProtocol(protocol) - if _, err := c.iaasClient.DefaultAPI.CreateSecurityGroupRule(ctx, c.projectID, c.region, nodeSecurityGroupID). - CreateSecurityGroupRulePayload(*rule). - Execute(); err != nil { - return shared.ClassifySDKError("create security group rule", err) - } - return nil -} - -func (c *SDKClient) findSecurityGroupByTags(ctx context.Context, tags map[string]string) (*SecurityGroup, error) { - matched, err := c.ListSecurityGroupsByTags(ctx, tags) - if err != nil { - return nil, err - } - if len(matched) == 0 { - return nil, fmt.Errorf("%w: no security group matching tags", ErrNotFound) - } - if len(matched) > 1 { - return nil, fmt.Errorf("%w: multiple security groups matching tags", ErrConflict) - } - return matched[0], nil -} - -// addSecurityGroupToServer attaches the group idempotently. Three outcomes are -// expected and must not fail the caller: -// - conflict: already attached -// - invalid input: the API rejects the duplicate ("Duplicate items in the list") -// - not found: the server has no network port yet, so there is nothing to -// attach to — the next reconcile retries once it does -func (c *SDKClient) addSecurityGroupToServer(ctx context.Context, serverID, securityGroupID string) error { - if err := c.iaasClient.DefaultAPI. - AddSecurityGroupToServer(ctx, c.projectID, c.region, serverID, securityGroupID). - Execute(); err != nil { - err := shared.ClassifySDKError("add security group to server", err) - if !IsConflict(err) && !IsInvalidInput(err) && !IsNotFound(err) { - return err - } - } - return nil -} - -func (c *SDKClient) deleteSecurityGroupRules(ctx context.Context, securityGroupID string) error { - resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, securityGroupID).Execute() - if err != nil { - err := shared.ClassifySDKError("list security group rules", err) - if IsNotFound(err) { - return nil - } - return err - } - for _, rule := range resp.GetItems() { - ruleID := rule.GetId() - if ruleID == "" { - continue - } - if err := c.iaasClient.DefaultAPI. - DeleteSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID, ruleID). - Execute(); err != nil { - err := shared.ClassifySDKError("delete security group rule", err) - if !IsNotFound(err) { - return err - } - } - } - return nil -} - -func hasSSHRule(rules []iaas.SecurityGroupRule, cidr string) bool { - for _, rule := range rules { - if rule.GetIpRange() == cidr && isSSHRule(rule) { - return true - } - } - return false -} - -// isSSHRule reports whether the rule is an ingress TCP/22 rule, regardless of -// which CIDR it allows. -func isSSHRule(rule iaas.SecurityGroupRule) bool { - if rule.GetDirection() != "ingress" { - return false - } - portRange := rule.GetPortRange() - if portRange.GetMin() != sshPort || portRange.GetMax() != sshPort { - return false - } - protocol, ok := rule.GetProtocolOk() - return ok && protocol.GetName() == tcpProtocol -} - -func hasRemoteSecurityGroupSSHRule(rules []iaas.SecurityGroupRule, remoteSecurityGroupID string) bool { - for _, rule := range rules { - if rule.GetDirection() != "ingress" || rule.GetRemoteSecurityGroupId() != remoteSecurityGroupID { - continue - } - portRange := rule.GetPortRange() - if portRange.GetMin() != sshPort || portRange.GetMax() != sshPort { - continue - } - if protocol, ok := rule.GetProtocolOk(); ok && protocol.GetName() == tcpProtocol { - return true - } - } - return false -} - func (c *SDKClient) updateAPIServerTargetPool( ctx context.Context, loadBalancerID string, @@ -900,16 +585,6 @@ func loadBalancerFromSDK(in *lb.LoadBalancer) *LoadBalancer { } } -func securityGroupFromSDK(in *iaas.SecurityGroup) *SecurityGroup { - if in == nil { - return nil - } - return &SecurityGroup{ - ID: in.GetId(), - Name: in.GetName(), - } -} - func listenerPort(listeners []lb.Listener) int32 { for _, listener := range listeners { if port := listener.GetPort(); port > 0 { @@ -919,20 +594,6 @@ func listenerPort(listeners []lb.Listener) int32 { return 0 } -func clusterServerSelectorFromTags(tags map[string]string) map[string]string { - selector := map[string]string{} - for _, key := range []string{ - "cluster.x-k8s.io/cluster-name", - "cluster.x-k8s.io/cluster-namespace", - "cluster.x-k8s.io/managed-by", - } { - if value := tags[key]; value != "" { - selector[key] = value - } - } - return selector -} - func isLoadBalancerServiceNotEnabled(err error) bool { return IsUnauthorized(err) && strings.Contains(err.Error(), "Service not enabled") } diff --git a/cloud/sdk_client_test.go b/cloud/sdk_client_test.go index 3e235b7..16eef07 100644 --- a/cloud/sdk_client_test.go +++ b/cloud/sdk_client_test.go @@ -482,10 +482,6 @@ func TestSDKClientEnsureBastionToleratesDuplicateSecurityGroupAttach(t *testing. "labels": map[string]any{"cluster": "test"}, }, }}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/rules"): - writeJSON(t, w, map[string]any{"items": []any{}}) - case r.Method == http.MethodPost && strings.HasSuffix(path, "/rules"): - writeJSON(t, w, map[string]any{"id": "77777777-7777-4777-8777-777777777777", "direction": "ingress"}) case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): writeJSON(t, w, map[string]any{"items": []any{}}) case r.Method == http.MethodPost && strings.HasSuffix(path, "/servers"): @@ -540,162 +536,42 @@ func TestSDKClientEnsureBastionToleratesDuplicateSecurityGroupAttach(t *testing. } } -// TestSDKClientEnsureBastionRevokesRemovedCIDR guards against security-group -// rules only ever being added. Narrowing allowedCIDRs must actually take -// access away from the previously allowed range. -func TestSDKClientEnsureBastionRevokesRemovedCIDR(t *testing.T) { - const staleRuleID = "55555555-5555-4555-8555-555555555555" +func TestSDKClientDeleteBastionDetachesSecurityGroupWithoutDeletingIt(t *testing.T) { var ( - createdRuleCIDRs []string - deletedRuleIDs []string + detachedServerID string + detachedSGID string + deletedServerID string ) server := newSDKTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path := r.URL.Path switch { - case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": testSDKSecurityGroup, "name": "bastion-ssh", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/rules"): - // One pre-existing SSH rule for the CIDR that is being revoked. - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": staleRuleID, - "direction": "ingress", - "ipRange": "198.51.100.0/24", - "portRange": map[string]any{"min": 22, "max": 22}, - "protocol": map[string]any{"name": "tcp"}, - }, - }}) - case r.Method == http.MethodPost && strings.HasSuffix(path, "/rules"): - payload := readJSON(t, r) - if cidr, ok := payload["ipRange"].(string); ok { - createdRuleCIDRs = append(createdRuleCIDRs, cidr) - } - writeJSON(t, w, map[string]any{"id": "77777777-7777-4777-8777-777777777777", "direction": "ingress"}) - case strings.Contains(path, "/security-groups/") && strings.Contains(path, "/servers/"): + case r.Method == http.MethodDelete && strings.Contains(path, "/servers/") && strings.Contains(path, "/security-groups/"): + parts := strings.Split(path, "/servers/") + sub := strings.Split(parts[1], "/security-groups/") + detachedServerID, detachedSGID = sub[0], sub[1] w.WriteHeader(http.StatusNoContent) - case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): - parts := strings.Split(strings.TrimSuffix(path, "/"), "/") - deletedRuleIDs = append(deletedRuleIDs, parts[len(parts)-1]) + case r.Method == http.MethodDelete && strings.HasSuffix(path, "/servers/"+testSDKServerID): + deletedServerID = testSDKServerID w.WriteHeader(http.StatusNoContent) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": testSDKServerID, "name": "bastion", "status": "ACTIVE", "machineType": "c2i.1", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/public-ips"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": "66666666-6666-4666-8666-666666666666", "ip": "203.0.113.10", "networkInterface": "nic-1", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.Contains(path, "/servers/"): - writeJSON(t, w, map[string]any{ - "id": testSDKServerID, "name": "bastion", "status": "ACTIVE", "machineType": "c2i.1", - }) default: t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) } })) client := newTestSDKClient(t, server.URL) - if _, err := client.EnsureBastion(context.Background(), BastionInput{ - Name: "bastion", - ProjectID: testSDKProjectID, - Region: testSDKRegion, - NetworkID: testSDKNetworkID, - ImageID: testSDKImageID, - MachineType: "c2i.1", - SSHKeyName: "default", - AllowedCIDRs: []string{"203.0.113.0/24"}, - Tags: map[string]string{"cluster": "test"}, - }); err != nil { - t.Fatalf("EnsureBastion() error = %v", err) - } - - if len(createdRuleCIDRs) != 1 || createdRuleCIDRs[0] != "203.0.113.0/24" { - t.Fatalf("created rules for %v, want [203.0.113.0/24]", createdRuleCIDRs) + err := client.DeleteBastion( + context.Background(), + BastionInput{Tags: map[string]string{"cluster": "test"}}, + Bastion{ServerID: testSDKServerID, SecurityGroupID: testSDKSecurityGroup}, + ) + if err != nil { + t.Fatalf("DeleteBastion() error = %v", err) } - if len(deletedRuleIDs) != 1 || deletedRuleIDs[0] != staleRuleID { - t.Fatalf("deleted rules %v, want [%s] — the revoked CIDR keeps its SSH access", deletedRuleIDs, staleRuleID) + if detachedServerID != testSDKServerID || detachedSGID != testSDKSecurityGroup { + t.Fatalf("detached (%q, %q), want (%q, %q)", detachedServerID, detachedSGID, testSDKServerID, testSDKSecurityGroup) } -} - -// TestSDKClientEnsureBastionDeduplicatesRepeatedCIDRs guards against a CIDR -// listed twice producing two identical rules: existingRules is a snapshot taken -// before the loop, so the second pass would not see the rule the first pass -// created and the duplicate create fails the whole reconcile. -func TestSDKClientEnsureBastionDeduplicatesRepeatedCIDRs(t *testing.T) { - var createdRuleCIDRs []string - server := newSDKTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - path := r.URL.Path - switch { - case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": testSDKSecurityGroup, "name": "bastion-ssh", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/rules"): - writeJSON(t, w, map[string]any{"items": []any{}}) - case r.Method == http.MethodPost && strings.HasSuffix(path, "/rules"): - payload := readJSON(t, r) - if cidr, ok := payload["ipRange"].(string); ok { - createdRuleCIDRs = append(createdRuleCIDRs, cidr) - } - writeJSON(t, w, map[string]any{"id": "77777777-7777-4777-8777-777777777777", "direction": "ingress"}) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": testSDKServerID, "name": "bastion", "status": "ACTIVE", "machineType": "c2i.1", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case strings.Contains(path, "/security-groups/") && strings.Contains(path, "/servers/"): - w.WriteHeader(http.StatusNoContent) - case r.Method == http.MethodGet && strings.HasSuffix(path, "/public-ips"): - writeJSON(t, w, map[string]any{"items": []any{ - map[string]any{ - "id": "66666666-6666-4666-8666-666666666666", "ip": "203.0.113.10", "networkInterface": "nic-1", - "labels": map[string]any{"cluster": "test"}, - }, - }}) - case r.Method == http.MethodGet && strings.Contains(path, "/servers/"): - writeJSON(t, w, map[string]any{ - "id": testSDKServerID, "name": "bastion", "status": "ACTIVE", "machineType": "c2i.1", - }) - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) - } - })) - - client := newTestSDKClient(t, server.URL) - if _, err := client.EnsureBastion(context.Background(), BastionInput{ - Name: "bastion", - ProjectID: testSDKProjectID, - Region: testSDKRegion, - NetworkID: testSDKNetworkID, - ImageID: testSDKImageID, - MachineType: "c2i.1", - SSHKeyName: "default", - // The same CIDR twice, plus a distinct one. - AllowedCIDRs: []string{"203.0.113.0/24", "203.0.113.0/24", "198.51.100.0/24"}, - Tags: map[string]string{"cluster": "test"}, - }); err != nil { - t.Fatalf("EnsureBastion() error = %v", err) - } - - if len(createdRuleCIDRs) != 2 { - t.Fatalf("created %d rules (%v), want 2 — the repeated CIDR was created twice", - len(createdRuleCIDRs), createdRuleCIDRs) + if deletedServerID != testSDKServerID { + t.Fatalf("deletedServerID = %q, want %q", deletedServerID, testSDKServerID) } } diff --git a/cloud/services/bastion/bastion.go b/cloud/services/bastion/bastion.go index 80eab63..d89ae2c 100644 --- a/cloud/services/bastion/bastion.go +++ b/cloud/services/bastion/bastion.go @@ -36,6 +36,7 @@ func Input(stackitCluster *infrav1.StackitCluster, cloudInit []byte) cloud.Basti AvailabilityZone: scope.DefaultAvailabilityZone(stackitCluster), SSHKeyName: stackitCluster.Spec.Bastion.SSHKeyName, AllowedCIDRs: stackitCluster.Spec.Bastion.AllowedCIDRs, + SecurityGroupID: stackitCluster.Status.Bastion.SecurityGroupID, Tags: tags, RootVolume: cloud.RootVolumeInput{ SizeGiB: stackitCluster.Spec.Bastion.RootVolume.SizeGiB, @@ -45,9 +46,3 @@ func Input(stackitCluster *infrav1.StackitCluster, cloudInit []byte) cloud.Basti CloudInit: cloudInit, } } - -func NodeSSHAccessTags(stackitCluster *infrav1.StackitCluster) map[string]string { - tags := util.ClusterTags(stackitCluster.Name, stackitCluster.Namespace, stackitCluster.Spec.AdditionalLabels) - tags[util.LabelResourceRole] = util.ResourceRoleNodeSSH - return tags -} diff --git a/cloud/services/securitygroup/reconcile.go b/cloud/services/securitygroup/reconcile.go new file mode 100644 index 0000000..75dbdc5 --- /dev/null +++ b/cloud/services/securitygroup/reconcile.go @@ -0,0 +1,323 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package securitygroup + +import ( + "context" + "fmt" + "slices" + + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" +) + +func (s *Service) Reconcile(ctx context.Context) error { + log := logf.FromContext(ctx) + for _, spec := range s.scope.SecurityGroupSpecs() { + if err := s.reconcileSecurityGroup(ctx, spec); err != nil { + return err + } + } + log.V(1).Info("Reconciled security group resources") + return nil +} + +func (s *Service) Delete(ctx context.Context) error { + log := logf.FromContext(ctx) + for _, spec := range slices.Backward(s.scope.SecurityGroupSpecs()) { + if err := s.deleteSecurityGroup(ctx, spec); err != nil { + return err + } + } + log.V(1).Info("Deleted security group resources") + return nil +} + +// FindByTags finds a single STACKIT security group matching the given tag selector. +func (c *Client) FindByTags(ctx context.Context, tags map[string]string) (*SecurityGroup, error) { + matched, err := c.ListByTags(ctx, tags) + if err != nil { + return nil, err + } + if len(matched) == 0 { + return nil, fmt.Errorf("%w: no security group matching tags", shared.ErrNotFound) + } + if len(matched) > 1 { + return nil, fmt.Errorf("%w: multiple security groups matching tags", shared.ErrConflict) + } + return matched[0], nil +} + +// ListByTags lists STACKIT security groups matching the given tag selector. +func (c *Client) ListByTags(ctx context.Context, tags map[string]string) ([]*SecurityGroup, error) { + if len(tags) == 0 { + return nil, fmt.Errorf("%w: empty tag selector", shared.ErrInvalidInput) + } + resp, err := c.iaasClient.DefaultAPI.ListSecurityGroups(ctx, c.projectID, c.region). + LabelSelector(shared.LabelSelector(tags)). + Execute() + if err != nil { + return nil, shared.ClassifySDKError("list security groups", err) + } + matched := []*SecurityGroup{} + for _, securityGroup := range resp.GetItems() { + if shared.LabelsContainTags(securityGroup.GetLabels(), tags) { + securityGroup := securityGroup + matched = append(matched, securityGroupFromSDK(&securityGroup)) + } + } + return matched, nil +} + +// DeleteByID deletes a security group and its rules by ID, ignoring NotFound errors. +func (c *Client) DeleteByID(ctx context.Context, id string) error { + if id == "" { + return fmt.Errorf("%w: security group ID is empty", shared.ErrInvalidInput) + } + rules, ok, err := c.listRulesIfGroupExists(ctx, id) + if err != nil || !ok { + return err + } + return c.deleteSecurityGroupWithRules(ctx, id, rules) +} + +// AttachToServer attaches a security group to a server idempotently. +// Three error responses are tolerated: +// - conflict: already attached +// - invalid input: duplicate item in security_groups list +// - not found: server has no network port yet +func (c *Client) AttachToServer(ctx context.Context, serverID, securityGroupID string) error { + if err := c.iaasClient.DefaultAPI. + AddSecurityGroupToServer(ctx, c.projectID, c.region, serverID, securityGroupID). + Execute(); err != nil { + err := shared.ClassifySDKError("add security group to server", err) + if !shared.IsConflict(err) && !shared.IsInvalidInput(err) && !shared.IsNotFound(err) { + return err + } + } + logf.FromContext(ctx).V(1).Info("Attached security group to server", "securityGroupID", securityGroupID, "serverID", serverID) + return nil +} + +// DetachFromServer detaches a security group from a server, ignoring NotFound and Conflict errors. +func (c *Client) DetachFromServer(ctx context.Context, serverID, securityGroupID string) error { + if err := c.iaasClient.DefaultAPI. + RemoveSecurityGroupFromServer(ctx, c.projectID, c.region, serverID, securityGroupID). + Execute(); err != nil { + err := shared.ClassifySDKError("remove security group from server", err) + if !shared.IsNotFound(err) && !shared.IsConflict(err) { + return err + } + } + logf.FromContext(ctx).V(1).Info("Detached security group from server", "securityGroupID", securityGroupID, "serverID", serverID) + return nil +} + +func (c *Client) reconcileSecurityGroup(ctx context.Context, spec SecurityGroupSpec) error { + if err := spec.validate(); err != nil { + return err + } + securityGroup, err := c.ensureSecurityGroup(ctx, spec) + if err != nil { + return err + } + if spec.SetID != nil { + spec.SetID(securityGroup.ID) + } + if err := c.ensureSecurityGroupRules(ctx, securityGroup.ID, spec.Rules); err != nil { + return err + } + for _, serverID := range spec.ServerIDs { + if serverID == "" { + continue + } + if err := c.AttachToServer(ctx, serverID, securityGroup.ID); err != nil { + return err + } + } + return nil +} + +func (c *Client) deleteSecurityGroup(ctx context.Context, spec SecurityGroupSpec) error { + if spec.ID == "" && len(spec.Tags) == 0 { + return fmt.Errorf("%w: security group ID or tags are required", shared.ErrInvalidInput) + } + + securityGroupID, rules, err := c.resolveSecurityGroupForDelete(ctx, spec) + if err != nil { + return err + } + if securityGroupID != "" { + if err := c.detachSecurityGroupFromServers(ctx, securityGroupID, spec); err != nil { + return err + } + if err := c.deleteSecurityGroupWithRules(ctx, securityGroupID, rules); err != nil { + return err + } + logf.FromContext(ctx).V(1).Info("Deleted security group", "securityGroupID", securityGroupID) + } + if spec.SetID != nil { + spec.SetID("") + } + return nil +} + +func (c *Client) resolveSecurityGroupForDelete( + ctx context.Context, + spec SecurityGroupSpec, +) (string, []iaas.SecurityGroupRule, error) { + if spec.ID != "" { + rules, ok, err := c.listRulesIfGroupExists(ctx, spec.ID) + if err != nil || ok { + return spec.ID, rules, err + } + } + if len(spec.Tags) == 0 { + return "", nil, nil + } + securityGroup, err := c.FindByTags(ctx, spec.Tags) + if shared.IsNotFound(err) { + return "", nil, nil + } + if err != nil { + return "", nil, err + } + rules, _, err := c.listRulesIfGroupExists(ctx, securityGroup.ID) + return securityGroup.ID, rules, err +} + +func (c *Client) listRulesIfGroupExists(ctx context.Context, id string) ([]iaas.SecurityGroupRule, bool, error) { + rules, err := c.listSecurityGroupRules(ctx, id) + if shared.IsNotFound(err) { + return nil, false, nil + } + if err != nil { + return nil, false, err + } + return rules, true, nil +} + +func (c *Client) ensureSecurityGroup(ctx context.Context, spec SecurityGroupSpec) (*SecurityGroup, error) { + if existing, err := c.FindByTags(ctx, spec.Tags); err == nil { + return existing, nil + } else if !shared.IsNotFound(err) { + return nil, err + } + securityGroup, err := c.iaasClient.DefaultAPI.CreateSecurityGroup(ctx, c.projectID, c.region). + CreateSecurityGroupPayload(spec.toCreatePayload()). + Execute() + if err != nil { + return nil, shared.ClassifySDKError("create security group", err) + } + out := securityGroupFromSDK(securityGroup) + log := logf.FromContext(ctx) + log.V(1).Info("Created security group", "name", spec.Name, "securityGroupID", out.ID) + return out, nil +} + +func (c *Client) deleteSecurityGroupWithRules( + ctx context.Context, + id string, + rules []iaas.SecurityGroupRule, +) error { + for _, rule := range rules { + if err := c.deleteSecurityGroupRule(ctx, id, rule.GetId()); err != nil { + return err + } + } + if err := c.iaasClient.DefaultAPI. + DeleteSecurityGroup(ctx, c.projectID, c.region, id). + Execute(); err != nil { + err := shared.ClassifySDKError("delete security group", err) + if !shared.IsNotFound(err) { + return err + } + } + return nil +} + +func (c *Client) detachSecurityGroupFromServers(ctx context.Context, securityGroupID string, spec SecurityGroupSpec) error { + if err := c.detachServers(ctx, securityGroupID, spec.ServerIDs); err != nil { + return err + } + if len(spec.ServerSelector) == 0 { + return nil + } + serverIDs, err := c.listServerIDsByTags(ctx, spec.ServerSelector) + if err != nil { + return err + } + return c.detachServers(ctx, securityGroupID, serverIDs) +} + +func (c *Client) detachServers(ctx context.Context, securityGroupID string, serverIDs []string) error { + for _, serverID := range serverIDs { + if serverID == "" { + continue + } + if err := c.DetachFromServer(ctx, serverID, securityGroupID); err != nil { + return err + } + } + return nil +} + +func (c *Client) listServerIDsByTags(ctx context.Context, tags map[string]string) ([]string, error) { + resp, err := c.iaasClient.DefaultAPI.ListServers(ctx, c.projectID, c.region). + LabelSelector(shared.LabelSelector(tags)). + Execute() + if err != nil { + return nil, shared.ClassifySDKError("list servers", err) + } + var serverIDs []string + for _, server := range resp.GetItems() { + if shared.LabelsContainTags(server.GetLabels(), tags) && server.GetId() != "" { + serverIDs = append(serverIDs, server.GetId()) + } + } + return serverIDs, nil +} + +func (s SecurityGroupSpec) validate() error { + if s.Name == "" { + return fmt.Errorf("%w: security group name is required", shared.ErrInvalidInput) + } + if len(s.Tags) == 0 { + return fmt.Errorf("%w: security group tags are required", shared.ErrInvalidInput) + } + for _, rule := range s.Rules { + if err := rule.validate(); err != nil { + return err + } + } + return nil +} + +func (s SecurityGroupSpec) toCreatePayload() iaas.CreateSecurityGroupPayload { + payload := iaas.NewCreateSecurityGroupPayload(s.Name) + if s.Description != "" { + payload.SetDescription(s.Description) + } + payload.SetLabels(shared.TagsToSDKLabels(s.Tags)) + return *payload +} + +func securityGroupFromSDK(in *iaas.SecurityGroup) *SecurityGroup { + if in == nil { + return nil + } + return &SecurityGroup{ + ID: in.GetId(), + Name: in.GetName(), + } +} diff --git a/cloud/services/securitygroup/reconcile_test.go b/cloud/services/securitygroup/reconcile_test.go new file mode 100644 index 0000000..3c44ad0 --- /dev/null +++ b/cloud/services/securitygroup/reconcile_test.go @@ -0,0 +1,743 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package securitygroup_test + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + + "github.com/stackitcloud/stackit-sdk-go/core/config" + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" +) + +const ( + testProjectID = "00000000-0000-0000-0000-000000000000" + testRegion = "eu01" + testSecurityGroupID = "11111111-1111-4111-8111-111111111111" + testRemoteSGID = "22222222-2222-4222-8222-222222222222" + testServerID = "33333333-3333-4333-8333-333333333333" + testRuleID1 = "66666666-6666-4666-8666-666666666666" + testRuleID2 = "77777777-7777-4777-8777-777777777777" +) + +type testScope struct { + projectID string + region string + iaasClient *iaas.APIClient + specs []securitygroup.SecurityGroupSpec +} + +func (s *testScope) ProjectID() string { + return s.projectID +} + +func (s *testScope) Region() string { + return s.region +} + +func (s *testScope) IaaSClient() *iaas.APIClient { + return s.iaasClient +} + +func (s *testScope) SecurityGroupSpecs() []securitygroup.SecurityGroupSpec { + return s.specs +} + +func newTestIaaSClient(t *testing.T, handler http.Handler) *iaas.APIClient { + t.Helper() + + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + + iaasClient, err := iaas.NewAPIClient( + config.WithoutAuthentication(), + config.WithEndpoint(server.URL), + ) + if err != nil { + t.Fatalf("iaas.NewAPIClient() error = %v", err) + } + return iaasClient +} + +func newTestService(t *testing.T, specs []securitygroup.SecurityGroupSpec, handler http.Handler) *securitygroup.Service { + t.Helper() + + return securitygroup.New(&testScope{ + projectID: testProjectID, + region: testRegion, + iaasClient: newTestIaaSClient(t, handler), + specs: specs, + }) +} + +func writeJSON(t *testing.T, w http.ResponseWriter, value any) { + t.Helper() + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(value); err != nil { + t.Fatalf("encode response body: %v", err) + } +} + +func readJSON(t *testing.T, r *http.Request) map[string]any { + t.Helper() + var out map[string]any + if err := json.NewDecoder(r.Body).Decode(&out); err != nil { + t.Fatalf("decode request body: %v", err) + } + return out +} + +func TestServiceReconcile(t *testing.T) { + t.Run("creates security group and generic rules when not found", func(t *testing.T) { + var ( + createdSGPayload map[string]any + createdRulePayloads []map[string]any + ) + specs := []securitygroup.SecurityGroupSpec{{ + Name: "cluster-sg", + Description: "Generic security group", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{ + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 6443, + PortMax: 6443, + IPRange: "203.0.113.0/24", + }, + { + Direction: securitygroup.DirectionEgress, + Protocol: securitygroup.ProtocolUDP, + PortMin: 8472, + PortMax: 8472, + RemoteSecurityGroupID: testRemoteSGID, + }, + }, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups"): + createdSGPayload = readJSON(t, r) + writeJSON(t, w, map[string]any{ + "id": testSecurityGroupID, + "name": "cluster-sg", + }) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + createdRulePayloads = append(createdRulePayloads, readJSON(t, r)) + writeJSON(t, w, map[string]any{"id": testRuleID1, "direction": "ingress"}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + + if createdSGPayload["name"] != "cluster-sg" { + t.Fatalf("created SG name = %v, want cluster-sg", createdSGPayload["name"]) + } + if createdSGPayload["description"] != "Generic security group" { + t.Fatalf("created SG description = %v, want Generic security group", createdSGPayload["description"]) + } + labels, _ := createdSGPayload["labels"].(map[string]any) + if labels["cluster"] != "test" { + t.Fatalf("created SG labels = %v, want cluster=test", labels) + } + if len(createdRulePayloads) != 2 { + t.Fatalf("created %d rules, want 2", len(createdRulePayloads)) + } + if createdRulePayloads[0]["direction"] != "ingress" || createdRulePayloads[0]["ipRange"] != "203.0.113.0/24" { + t.Fatalf("first rule payload = %#v", createdRulePayloads[0]) + } + if createdRulePayloads[1]["direction"] != "egress" || createdRulePayloads[1]["remoteSecurityGroupId"] != testRemoteSGID { + t.Fatalf("second rule payload = %#v", createdRulePayloads[1]) + } + }) + + t.Run("reuses existing security group found by tags without creating duplicate", func(t *testing.T) { + specs := []securitygroup.SecurityGroupSpec{{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testSecurityGroupID, + "name": "cluster-sg", + "labels": map[string]any{"cluster": "test"}, + }, + }}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{}}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + }) + + t.Run("returns ErrConflict when multiple security groups match tags", func(t *testing.T) { + specs := []securitygroup.SecurityGroupSpec{{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testSecurityGroupID, "name": "sg-1", "labels": map[string]any{"cluster": "test"}}, + map[string]any{"id": testRemoteSGID, "name": "sg-2", "labels": map[string]any{"cluster": "test"}}, + }}) + })) + + err := svc.Reconcile(context.Background()) + if !errors.Is(err, shared.ErrConflict) { + t.Fatalf("Reconcile() error = %v, want ErrConflict", err) + } + }) +} + +func TestServiceReconcileBastionAndNodeSSH(t *testing.T) { + var ( + bastionSGID string + bastionCreated bool + nodeRulesCreated []map[string]any + attachedServerIDs []string + ) + specs := []securitygroup.SecurityGroupSpec{ + { + Name: "cluster-bastion-ssh", + Tags: map[string]string{"role": "bastion"}, + SetID: func(id string) { + bastionSGID = id + }, + Rules: []securitygroup.Rule{ + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + IPRange: "203.0.113.0/24", + }, + { + Direction: securitygroup.DirectionEgress, + }, + }, + }, + { + Name: "cluster-node-ssh", + Tags: map[string]string{"role": "node-ssh"}, + ServerIDs: []string{testServerID}, + Rules: []securitygroup.Rule{ + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + RemoteSecurityGroupTags: map[string]string{"role": "bastion"}, + }, + { + Direction: securitygroup.DirectionEgress, + }, + }, + }, + } + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + selector := r.URL.Query().Get("label_selector") + if bastionCreated && strings.Contains(selector, "role=bastion") { + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testRemoteSGID, + "name": "cluster-bastion-ssh", + "labels": map[string]any{"role": "bastion"}, + }, + }}) + return + } + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups"): + payload := readJSON(t, r) + if payload["name"] == "cluster-bastion-ssh" { + bastionCreated = true + writeJSON(t, w, map[string]any{"id": testRemoteSGID, "name": "cluster-bastion-ssh"}) + return + } + writeJSON(t, w, map[string]any{"id": testSecurityGroupID, "name": "cluster-node-ssh"}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testRemoteSGID+"/rules"): + // Simulate STACKIT default egress rule already present. + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRuleID1, "direction": "egress", "ethertype": "IPv4"}, + }}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups/"+testRemoteSGID+"/rules"): + writeJSON(t, w, map[string]any{"id": testRuleID2, "direction": "ingress"}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + nodeRulesCreated = append(nodeRulesCreated, readJSON(t, r)) + writeJSON(t, w, map[string]any{"id": testRuleID1, "direction": "ingress"}) + case r.Method == http.MethodPut && strings.HasSuffix(path, "/servers/"+testServerID+"/security-groups/"+testSecurityGroupID): + attachedServerIDs = append(attachedServerIDs, testServerID) + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + if bastionSGID != testRemoteSGID { + t.Fatalf("bastionSGID = %q, want %q", bastionSGID, testRemoteSGID) + } + if len(nodeRulesCreated) != 2 { + t.Fatalf("nodeRulesCreated = %#v, want 2 rules (ingress from bastion + egress)", nodeRulesCreated) + } + if nodeRulesCreated[0]["remoteSecurityGroupId"] != testRemoteSGID { + t.Fatalf("node ingress rule remoteSecurityGroupId = %v, want %q", nodeRulesCreated[0]["remoteSecurityGroupId"], testRemoteSGID) + } + if nodeRulesCreated[1]["direction"] != "egress" { + t.Fatalf("node egress rule direction = %v, want egress", nodeRulesCreated[1]["direction"]) + } + if !slices.Equal(attachedServerIDs, []string{testServerID}) { + t.Fatalf("attachedServerIDs = %v, want [%s]", attachedServerIDs, testServerID) + } +} + +func TestServiceReconcileRules(t *testing.T) { + var ( + createdRules []map[string]any + deletedRuleIDs []string + ) + specs := []securitygroup.SecurityGroupSpec{{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{ + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 443, + PortMax: 443, + IPRange: "203.0.113.0/24", + }, + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 443, + PortMax: 443, + IPRange: "203.0.113.0/24", + }, + { + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 10250, + PortMax: 10250, + RemoteSecurityGroupID: testRemoteSGID, + }, + }, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testSecurityGroupID, + "name": "cluster-sg", + "labels": map[string]any{"cluster": "test"}, + }, + }}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testRuleID1, + "direction": "ingress", + "remoteSecurityGroupId": testRemoteSGID, + "portRange": map[string]any{"min": 10250, "max": 10250}, + "protocol": map[string]any{"name": "tcp"}, + }, + map[string]any{ + "id": testRuleID2, + "direction": "ingress", + "ipRange": "198.51.100.0/24", + "portRange": map[string]any{"min": 22, "max": 22}, + "protocol": map[string]any{"name": "tcp"}, + }, + }}) + case r.Method == http.MethodPost && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + createdRules = append(createdRules, readJSON(t, r)) + writeJSON(t, w, map[string]any{"id": "88888888-8888-4888-8888-888888888888", "direction": "ingress"}) + case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): + parts := strings.Split(strings.TrimSuffix(path, "/"), "/") + deletedRuleIDs = append(deletedRuleIDs, parts[len(parts)-1]) + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Reconcile(context.Background()); err != nil { + t.Fatalf("Reconcile() error = %v", err) + } + + if len(createdRules) != 1 || createdRules[0]["ipRange"] != "203.0.113.0/24" { + t.Fatalf("createdRules = %#v, want single rule for 203.0.113.0/24", createdRules) + } + if !slices.Equal(deletedRuleIDs, []string{testRuleID2}) { + t.Fatalf("deletedRuleIDs = %v, want [%s]", deletedRuleIDs, testRuleID2) + } +} + +func TestServiceReconcileValidation(t *testing.T) { + tests := []struct { + name string + spec securitygroup.SecurityGroupSpec + }{ + { + name: "missing name", + spec: securitygroup.SecurityGroupSpec{ + Tags: map[string]string{"cluster": "test"}, + }, + }, + { + name: "missing tags", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + }, + }, + { + name: "invalid rule direction", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{Direction: "invalid", Protocol: securitygroup.ProtocolTCP, PortMin: 80, PortMax: 80, IPRange: "0.0.0.0/0"}}, + }, + }, + { + name: "both ipRange and remoteSecurityGroupID set", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 80, + PortMax: 80, + IPRange: "0.0.0.0/0", + RemoteSecurityGroupID: testRemoteSGID, + }}, + }, + }, + { + name: "both ipRange and remoteSecurityGroupTags set", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 80, + PortMax: 80, + IPRange: "0.0.0.0/0", + RemoteSecurityGroupTags: map[string]string{"role": "bastion"}, + }}, + }, + }, + { + name: "invalid port range", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{Direction: securitygroup.DirectionIngress, Protocol: securitygroup.ProtocolTCP, PortMin: 8080, PortMax: 80, IPRange: "0.0.0.0/0"}}, + }, + }, + { + name: "port range without protocol", + spec: securitygroup.SecurityGroupSpec{ + Name: "cluster-sg", + Tags: map[string]string{"cluster": "test"}, + Rules: []securitygroup.Rule{{Direction: securitygroup.DirectionIngress, PortMin: 80, PortMax: 80, IPRange: "0.0.0.0/0"}}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc := newTestService(t, []securitygroup.SecurityGroupSpec{tt.spec}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("unexpected request before validation failure: %s %s", r.Method, r.URL.String()) + })) + + err := svc.Reconcile(context.Background()) + if !errors.Is(err, shared.ErrInvalidInput) { + t.Fatalf("Reconcile() error = %v, want ErrInvalidInput", err) + } + }) + } +} + +func TestServiceDelete(t *testing.T) { + t.Run("deletes rules and security group using ID when provided", func(t *testing.T) { + var ( + deletedRules []string + deletedSGID string + ) + specs := []securitygroup.SecurityGroupSpec{{ + ID: testSecurityGroupID, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRuleID1, "direction": "ingress"}, + map[string]any{"id": testRuleID2, "direction": "egress"}, + }}) + case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): + parts := strings.Split(strings.TrimSuffix(path, "/"), "/") + deletedRules = append(deletedRules, parts[len(parts)-1]) + w.WriteHeader(http.StatusNoContent) + case r.Method == http.MethodDelete && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID): + deletedSGID = testSecurityGroupID + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Delete(context.Background()); err != nil { + t.Fatalf("Delete() error = %v", err) + } + if !slices.Equal(deletedRules, []string{testRuleID1, testRuleID2}) { + t.Fatalf("deletedRules = %v, want [%s %s]", deletedRules, testRuleID1, testRuleID2) + } + if deletedSGID != testSecurityGroupID { + t.Fatalf("deletedSGID = %q, want %q", deletedSGID, testSecurityGroupID) + } + }) + + t.Run("deletes rules and security group found by tags", func(t *testing.T) { + var ( + deletedRules []string + deletedSGID string + ) + specs := []securitygroup.SecurityGroupSpec{{ + Tags: map[string]string{"cluster": "test"}, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testSecurityGroupID, "name": "cluster-sg", "labels": map[string]any{"cluster": "test"}}, + }}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRuleID1, "direction": "ingress"}, + map[string]any{"id": testRuleID2, "direction": "egress"}, + }}) + case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): + parts := strings.Split(strings.TrimSuffix(path, "/"), "/") + deletedRules = append(deletedRules, parts[len(parts)-1]) + w.WriteHeader(http.StatusNoContent) + case r.Method == http.MethodDelete && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID): + deletedSGID = testSecurityGroupID + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Delete(context.Background()); err != nil { + t.Fatalf("Delete() error = %v", err) + } + if !slices.Equal(deletedRules, []string{testRuleID1, testRuleID2}) { + t.Fatalf("deletedRules = %v, want [%s %s]", deletedRules, testRuleID1, testRuleID2) + } + if deletedSGID != testSecurityGroupID { + t.Fatalf("deletedSGID = %q, want %q", deletedSGID, testSecurityGroupID) + } + }) + + t.Run("succeeds when security group does not exist", func(t *testing.T) { + specs := []securitygroup.SecurityGroupSpec{{ + Tags: map[string]string{"cluster": "test"}, + }} + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, map[string]any{"items": []any{}}) + })) + + if err := svc.Delete(context.Background()); err != nil { + t.Fatalf("Delete() error = %v, want nil", err) + } + }) + + t.Run("returns ErrInvalidInput when tags are empty", func(t *testing.T) { + svc := newTestService(t, []securitygroup.SecurityGroupSpec{{}}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + })) + + err := svc.Delete(context.Background()) + if !errors.Is(err, shared.ErrInvalidInput) { + t.Fatalf("Delete() error = %v, want ErrInvalidInput", err) + } + }) +} + +func TestServiceDeleteStaleIDFallbackAndDetach(t *testing.T) { + var ( + detachedPairs []string + deletedSGIDs []string + ) + specs := []securitygroup.SecurityGroupSpec{ + { + ID: "99999999-9999-4999-8999-999999999999", + Tags: map[string]string{"role": "bastion"}, + }, + { + Tags: map[string]string{"role": "node-ssh"}, + ServerSelector: map[string]string{"cluster": "test"}, + }, + } + + svc := newTestService(t, specs, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/99999999-9999-4999-8999-999999999999/rules"): + w.WriteHeader(http.StatusNotFound) + writeJSON(t, w, map[string]any{"message": "not found"}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups"): + selector := r.URL.Query().Get("label_selector") + if strings.Contains(selector, "role=node-ssh") { + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testSecurityGroupID, "name": "node-ssh", "labels": map[string]any{"role": "node-ssh"}}, + }}) + return + } + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRemoteSGID, "name": "bastion-ssh", "labels": map[string]any{"role": "bastion"}}, + }}) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/servers"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{ + "id": testServerID, + "name": "worker-1", + "machineType": "c2i.1", + "labels": map[string]any{"cluster": "test"}, + }, + }}) + case r.Method == http.MethodDelete && strings.Contains(path, "/servers/") && strings.Contains(path, "/security-groups/"): + parts := strings.Split(path, "/servers/") + sub := strings.Split(parts[1], "/security-groups/") + detachedPairs = append(detachedPairs, sub[0]+":"+sub[1]) + w.WriteHeader(http.StatusNoContent) + case r.Method == http.MethodGet && strings.HasSuffix(path, "/rules"): + writeJSON(t, w, map[string]any{"items": []any{}}) + case r.Method == http.MethodDelete && strings.Contains(path, "/security-groups/"): + id := path[strings.LastIndex(path, "/security-groups/")+len("/security-groups/"):] + deletedSGIDs = append(deletedSGIDs, id) + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.Delete(context.Background()); err != nil { + t.Fatalf("Delete() error = %v", err) + } + if !slices.Equal(detachedPairs, []string{testServerID + ":" + testSecurityGroupID}) { + t.Fatalf("detachedPairs = %v, want [%s:%s]", detachedPairs, testServerID, testSecurityGroupID) + } + if !slices.Equal(deletedSGIDs, []string{testSecurityGroupID, testRemoteSGID}) { + t.Fatalf("deletedSGIDs = %v, want [%s %s] (reverse order: node-ssh then bastion-ssh)", deletedSGIDs, testSecurityGroupID, testRemoteSGID) + } +} + +func TestFindByTagsAndDeleteByID(t *testing.T) { + t.Run("FindByTags returns matching security group", func(t *testing.T) { + svc := newTestService(t, nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testSecurityGroupID, "name": "bastion-ssh", "labels": map[string]any{"cluster": "test"}}, + }}) + })) + + got, err := svc.FindByTags(context.Background(), map[string]string{"cluster": "test"}) + if err != nil { + t.Fatalf("FindByTags() error = %v", err) + } + if got.ID != testSecurityGroupID { + t.Fatalf("FindByTags().ID = %q, want %q", got.ID, testSecurityGroupID) + } + }) + + t.Run("DeleteByID deletes rules and security group and ignores NotFound", func(t *testing.T) { + var ( + deletedRules []string + deletedSGID string + ) + svc := newTestService(t, nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case r.Method == http.MethodGet && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID+"/rules"): + writeJSON(t, w, map[string]any{"items": []any{ + map[string]any{"id": testRuleID1, "direction": "ingress"}, + }}) + case r.Method == http.MethodDelete && strings.Contains(path, "/rules/"): + parts := strings.Split(strings.TrimSuffix(path, "/"), "/") + deletedRules = append(deletedRules, parts[len(parts)-1]) + w.WriteHeader(http.StatusNoContent) + case r.Method == http.MethodDelete && strings.HasSuffix(path, "/security-groups/"+testSecurityGroupID): + deletedSGID = testSecurityGroupID + w.WriteHeader(http.StatusNoContent) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + + if err := svc.DeleteByID(context.Background(), testSecurityGroupID); err != nil { + t.Fatalf("DeleteByID() error = %v", err) + } + if !slices.Equal(deletedRules, []string{testRuleID1}) { + t.Fatalf("deletedRules = %v, want [%s]", deletedRules, testRuleID1) + } + if deletedSGID != testSecurityGroupID { + t.Fatalf("deletedSGID = %q, want %q", deletedSGID, testSecurityGroupID) + } + }) +} diff --git a/cloud/services/securitygroup/rules.go b/cloud/services/securitygroup/rules.go new file mode 100644 index 0000000..901f585 --- /dev/null +++ b/cloud/services/securitygroup/rules.go @@ -0,0 +1,198 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package securitygroup + +import ( + "context" + "fmt" + "slices" + "strings" + + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" +) + +func (c *Client) ensureSecurityGroupRules(ctx context.Context, securityGroupID string, rules []Rule) error { + if securityGroupID == "" { + return fmt.Errorf("%w: security group ID is required", shared.ErrInvalidInput) + } + existingRules, err := c.listSecurityGroupRules(ctx, securityGroupID) + if err != nil { + return err + } + desired := make([]Rule, 0, len(rules)) + for _, rule := range rules { + rule, err := c.resolveRule(ctx, rule) + if err != nil { + return err + } + if slices.ContainsFunc(desired, rule.equal) { + continue + } + desired = append(desired, rule) + if hasRule(existingRules, rule) { + continue + } + if err := c.createSecurityGroupRule(ctx, securityGroupID, rule); err != nil { + return err + } + } + + for _, existing := range existingRules { + if slices.ContainsFunc(desired, ruleFromSDK(existing).equal) { + continue + } + if err := c.deleteSecurityGroupRule(ctx, securityGroupID, existing.GetId()); err != nil { + return err + } + } + return nil +} + +func (c *Client) resolveRule(ctx context.Context, rule Rule) (Rule, error) { + rule = rule.normalize() + if rule.RemoteSecurityGroupID == "" && len(rule.RemoteSecurityGroupTags) > 0 { + remoteSG, err := c.FindByTags(ctx, rule.RemoteSecurityGroupTags) + if err != nil { + return Rule{}, err + } + rule.RemoteSecurityGroupID = remoteSG.ID + } + rule.RemoteSecurityGroupTags = nil + return rule, nil +} + +func (c *Client) createSecurityGroupRule(ctx context.Context, securityGroupID string, rule Rule) error { + if _, err := c.iaasClient.DefaultAPI.CreateSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID). + CreateSecurityGroupRulePayload(rule.toCreatePayload()). + Execute(); err != nil { + return shared.ClassifySDKError("create security group rule", err) + } + return nil +} + +func (c *Client) listSecurityGroupRules(ctx context.Context, securityGroupID string) ([]iaas.SecurityGroupRule, error) { + resp, err := c.iaasClient.DefaultAPI.ListSecurityGroupRules(ctx, c.projectID, c.region, securityGroupID).Execute() + if err != nil { + return nil, shared.ClassifySDKError("list security group rules", err) + } + return resp.GetItems(), nil +} + +func (c *Client) deleteSecurityGroupRule(ctx context.Context, securityGroupID, ruleID string) error { + if ruleID == "" { + return nil + } + if err := c.iaasClient.DefaultAPI. + DeleteSecurityGroupRule(ctx, c.projectID, c.region, securityGroupID, ruleID). + Execute(); err != nil { + err := shared.ClassifySDKError("delete security group rule", err) + if !shared.IsNotFound(err) { + return err + } + } + return nil +} + +func (r Rule) validate() error { + if r.Direction != DirectionIngress && r.Direction != DirectionEgress { + return fmt.Errorf( + "%w: security group rule direction must be %q or %q", + shared.ErrInvalidInput, + DirectionIngress, + DirectionEgress, + ) + } + if r.IPRange != "" && (r.RemoteSecurityGroupID != "" || len(r.RemoteSecurityGroupTags) > 0) { + return fmt.Errorf( + "%w: security group rule cannot specify both ipRange and remoteSecurityGroupID", + shared.ErrInvalidInput, + ) + } + if r.hasInvalidPortRange() { + return fmt.Errorf("%w: invalid security group rule port range", shared.ErrInvalidInput) + } + if r.hasPortRange() && r.Protocol == "" { + return fmt.Errorf("%w: security group rule port range requires a protocol", shared.ErrInvalidInput) + } + return nil +} + +func (r Rule) hasPortRange() bool { + return r.PortMin > 0 || r.PortMax > 0 +} + +func (r Rule) hasInvalidPortRange() bool { + return r.PortMin < 0 || r.PortMax < 0 || r.PortMin > r.PortMax +} + +func (p Protocol) normalize() Protocol { + return Protocol(strings.ToLower(string(p))) +} + +func (r Rule) normalize() Rule { + r.Protocol = r.Protocol.normalize() + return r +} + +func (r Rule) equal(other Rule) bool { + return r.Direction == other.Direction && + r.Protocol == other.Protocol && + r.PortMin == other.PortMin && + r.PortMax == other.PortMax && + r.IPRange == other.IPRange && + r.RemoteSecurityGroupID == other.RemoteSecurityGroupID +} + +func (r Rule) toCreatePayload() iaas.CreateSecurityGroupRulePayload { + payload := iaas.NewCreateSecurityGroupRulePayload(string(r.Direction)) + if r.Protocol != "" { + protocol := iaas.StringAsCreateProtocol(new(string(r.Protocol))) + payload.SetProtocol(protocol) + } + if r.hasPortRange() { + payload.SetPortRange(*iaas.NewPortRange(r.PortMin, r.PortMax)) + } + if r.IPRange != "" { + payload.SetIpRange(r.IPRange) + } + if r.RemoteSecurityGroupID != "" { + payload.SetRemoteSecurityGroupId(r.RemoteSecurityGroupID) + } + return *payload +} + +func hasRule(rules []iaas.SecurityGroupRule, desired Rule) bool { + return slices.ContainsFunc(rules, func(rule iaas.SecurityGroupRule) bool { + return ruleFromSDK(rule).equal(desired) + }) +} + +func ruleFromSDK(in iaas.SecurityGroupRule) Rule { + var ruleProtocol Protocol + if protocol, ok := in.GetProtocolOk(); ok { + ruleProtocol = Protocol(protocol.GetName()).normalize() + } + var portMin, portMax int64 + if portRange, ok := in.GetPortRangeOk(); ok { + portMin = portRange.GetMin() + portMax = portRange.GetMax() + } + return Rule{ + Direction: Direction(in.GetDirection()), + Protocol: ruleProtocol, + PortMin: portMin, + PortMax: portMax, + IPRange: in.GetIpRange(), + RemoteSecurityGroupID: in.GetRemoteSecurityGroupId(), + } +} diff --git a/cloud/services/securitygroup/service.go b/cloud/services/securitygroup/service.go new file mode 100644 index 0000000..584227a --- /dev/null +++ b/cloud/services/securitygroup/service.go @@ -0,0 +1,102 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package securitygroup + +import iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + +// Direction specifies the traffic direction of a security group rule. +type Direction string + +const ( + DirectionIngress Direction = "ingress" + DirectionEgress Direction = "egress" +) + +// Protocol specifies the network protocol of a security group rule. +type Protocol string + +const ( + ProtocolTCP Protocol = "tcp" + ProtocolUDP Protocol = "udp" + ProtocolICMP Protocol = "icmp" +) + +// SecurityGroup describes a STACKIT security group. +type SecurityGroup struct { + ID string + Name string +} + +// Rule describes a desired STACKIT security group rule. +type Rule struct { + Direction Direction + Protocol Protocol + PortMin int64 + PortMax int64 + IPRange string + RemoteSecurityGroupID string + RemoteSecurityGroupTags map[string]string +} + +// SecurityGroupSpec describes the desired state of a STACKIT security group and its rules. +type SecurityGroupSpec struct { + ID string + Name string + Description string + Tags map[string]string + Rules []Rule + ServerIDs []string + ServerSelector map[string]string + SetID func(string) +} + +// ClientScope is an interface that provides the IaaS client and target scope for security group operations. +type ClientScope interface { + ProjectID() string + Region() string + IaaSClient() *iaas.APIClient +} + +// Scope is an interface that provides scope data needed for security group reconciliation. +type Scope interface { + ClientScope + SecurityGroupSpecs() []SecurityGroupSpec +} + +// Client provides direct security group operations against the STACKIT IaaS API. +type Client struct { + iaasClient *iaas.APIClient + projectID string + region string +} + +// NewClient returns a Client for the given scope. +func NewClient(scope ClientScope) *Client { + return &Client{ + iaasClient: scope.IaaSClient(), + projectID: scope.ProjectID(), + region: scope.Region(), + } +} + +// Service implements security groups reconciler. +type Service struct { + scope Scope + *Client +} + +// New returns Service from given scope. +func New(scope Scope) *Service { + return &Service{ + scope: scope, + Client: NewClient(scope), + } +} diff --git a/cloud/types.go b/cloud/types.go index 6cca29a..98b9fda 100644 --- a/cloud/types.go +++ b/cloud/types.go @@ -15,6 +15,7 @@ package cloud import ( "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/shared" ) @@ -44,10 +45,7 @@ type LoadBalancer struct { type PublicIP = publicip.PublicIP // SecurityGroup describes a STACKIT security group. -type SecurityGroup struct { - ID string - Name string -} +type SecurityGroup = securitygroup.SecurityGroup // Bastion describes the provider-managed SSH bastion resources. type Bastion struct { @@ -84,20 +82,12 @@ type BastionInput struct { AvailabilityZone string SSHKeyName string AllowedCIDRs []string + SecurityGroupID string Tags map[string]string RootVolume RootVolumeInput CloudInit []byte } -// NodeSSHAccessInput holds parameters required to allow SSH from the bastion -// security group to cluster nodes. -type NodeSSHAccessInput struct { - Name string - ServerID string - BastionSecurityGroupID string - Tags map[string]string -} - // RootVolumeInput describes the root disk of a VM. type RootVolumeInput struct { SizeGiB int diff --git a/controller/stackitcluster_bastion.go b/controller/stackitcluster_bastion.go index e2c1a1a..8c7d888 100644 --- a/controller/stackitcluster_bastion.go +++ b/controller/stackitcluster_bastion.go @@ -30,6 +30,7 @@ import ( "github.com/stackitcloud/cluster-api-provider-stackit/cloud" bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/scope" ) @@ -115,13 +116,21 @@ func (r *StackitClusterReconciler) reconcileBastion( return ctrl.Result{RequeueAfter: retryableErrorRequeueAfter}, false, nil } - hadBastionStatus := hasBastionStatus(stackitCluster.Status.Bastion) + hadBastionServer := stackitCluster.Status.Bastion.ServerID != "" + // Temporary ordering until bastion server reconciliation is extracted into a + // service: ensure the security group first so its ID can be attached to the + // bastion server by EnsureBastion. + if err := securitygroup.New(clusterScope).Reconcile(ctx); err != nil { + return ctrl.Result{}, false, err + } + input.SecurityGroupID = stackitCluster.Status.Bastion.SecurityGroupID + bastion, err := cloudClient.EnsureBastion(ctx, input) if err != nil { return ctrl.Result{}, false, err } clusterScope.SetBastionStatus(bastion, bastionCloudInitHash(cloudInit)) - if !hadBastionStatus { + if !hadBastionServer { record.Eventf( stackitCluster, "BastionCreated", "Created bastion %s", bastion.ServerID, ) @@ -192,17 +201,20 @@ func (r *StackitClusterReconciler) deleteBastionResources( status cloud.Bastion, ignoreNotFound bool, ) error { - if err := cloudClient.DeleteNodeSSHAccess(ctx, bastionservice.NodeSSHAccessTags(clusterScope.StackitCluster)); err != nil { + if err := publicip.New(clusterScope).Delete(ctx); err != nil { if !ignoreNotFound || !cloud.IsNotFound(err) { return err } } - if err := publicip.New(clusterScope).Delete(ctx); err != nil { + if err := cloudClient.DeleteBastion(ctx, input, status); err != nil { if !ignoreNotFound || !cloud.IsNotFound(err) { return err } } - if err := cloudClient.DeleteBastion(ctx, input, status); err != nil { + // Temporary ordering until bastion server reconciliation is extracted into a + // service: the security group must be deleted after the server is detached + // and deleted by DeleteBastion. + if err := securitygroup.New(clusterScope).Delete(ctx); err != nil { if !ignoreNotFound || !cloud.IsNotFound(err) { return err } @@ -211,7 +223,7 @@ func (r *StackitClusterReconciler) deleteBastionResources( } func bastionNeedsRecreate(stackitCluster *infrav1.StackitCluster, cloudInit []byte) bool { - if !hasBastionStatus(stackitCluster.Status.Bastion) { + if stackitCluster.Status.Bastion.ServerID == "" { return false } return stackitCluster.Status.Bastion.CloudInitHash != bastionCloudInitHash(cloudInit) diff --git a/controller/stackitcluster_controller_test.go b/controller/stackitcluster_controller_test.go index ccd6cf1..3898f6d 100644 --- a/controller/stackitcluster_controller_test.go +++ b/controller/stackitcluster_controller_test.go @@ -28,7 +28,9 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" cloudfake "github.com/stackitcloud/cluster-api-provider-stackit/cloud/fake" - bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" + "github.com/stackitcloud/cluster-api-provider-stackit/scope" + "github.com/stackitcloud/cluster-api-provider-stackit/util" ) var _ = Describe("StackitCluster Controller", func() { @@ -135,7 +137,34 @@ var _ = Describe("StackitCluster Controller", func() { Expect(got.Status.Bastion.SecurityGroupID).NotTo(BeEmpty()) Expect(fakeCloud.ServerCount()).To(Equal(1)) Expect(fakeCloud.PublicIPCount()).To(Equal(1)) - Expect(fakeCloud.SecurityGroupCount()).To(Equal(1)) + Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) + Expect(fakeCloud.SecurityGroupRules(got.Status.Bastion.SecurityGroupID)).To(ConsistOf( + securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + IPRange: "203.0.113.10/32", + }, + securitygroup.Rule{ + Direction: securitygroup.DirectionEgress, + }, + )) + nodeSGs, err := fakeCloud.ListSecurityGroupsByTags(ctx, scope.NodeSSHTags(got)) + Expect(err).NotTo(HaveOccurred()) + Expect(nodeSGs).To(HaveLen(1)) + Expect(fakeCloud.SecurityGroupRules(nodeSGs[0].ID)).To(ConsistOf( + securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + RemoteSecurityGroupID: got.Status.Bastion.SecurityGroupID, + }, + securitygroup.Rule{ + Direction: securitygroup.DirectionEgress, + }, + )) expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionTrue, "Available") }) @@ -169,14 +198,18 @@ var _ = Describe("StackitCluster Controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) - _, err = fakeCloud.EnsureNodeSSHAccess(ctx, cloud.NodeSSHAccessInput{ - Name: got.Name + "-node-ssh", - ServerID: got.Status.Bastion.ServerID, - BastionSecurityGroupID: got.Status.Bastion.SecurityGroupID, - Tags: bastionservice.NodeSSHAccessTags(got), + Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) + nodeSGs, err := fakeCloud.ListSecurityGroupsByTags(ctx, scope.NodeSSHTags(got)) + Expect(err).NotTo(HaveOccurred()) + Expect(nodeSGs).To(HaveLen(1)) + workerServer, err := fakeCloud.CreateServer(ctx, cloud.CreateServerInput{ + Name: got.Name + "-worker-0", + Tags: util.MachineTags(got.Name, got.Namespace, "worker-0", "uid-1", nil), + SecurityGroups: []string{nodeSGs[0].ID}, }) Expect(err).NotTo(HaveOccurred()) - Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) + Expect(fakeCloud.ServerHasSecurityGroup(workerServer.ID, nodeSGs[0].ID)).To(BeTrue()) + got.Spec.Bastion.Enabled = false Expect(k8sClient.Update(ctx, got)).To(Succeed()) @@ -186,7 +219,8 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(got.Status.Bastion).To(Equal(infrav1.StackitBastionStatus{})) - Expect(fakeCloud.ServerCount()).To(Equal(0)) + Expect(fakeCloud.ServerHasSecurityGroup(workerServer.ID, nodeSGs[0].ID)).To(BeFalse()) + Expect(fakeCloud.ServerCount()).To(Equal(1)) Expect(fakeCloud.PublicIPCount()).To(Equal(0)) Expect(fakeCloud.SecurityGroupCount()).To(Equal(0)) expectCondition(got.Status.Conditions, infrav1.ClusterBastionReadyCondition, metav1.ConditionTrue, "Skipped") @@ -202,13 +236,6 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(got.Status.Bastion.PublicIP).NotTo(BeEmpty()) - _, err = fakeCloud.EnsureNodeSSHAccess(ctx, cloud.NodeSSHAccessInput{ - Name: got.Name + "-node-ssh", - ServerID: got.Status.Bastion.ServerID, - BastionSecurityGroupID: got.Status.Bastion.SecurityGroupID, - Tags: bastionservice.NodeSSHAccessTags(got), - }) - Expect(err).NotTo(HaveOccurred()) Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) Expect(k8sClient.Delete(ctx, got)).To(Succeed()) @@ -450,13 +477,6 @@ var _ = Describe("StackitCluster Controller", func() { Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) oldServerID := got.Status.Bastion.ServerID Expect(oldServerID).NotTo(BeEmpty()) - _, err = fakeCloud.EnsureNodeSSHAccess(ctx, cloud.NodeSSHAccessInput{ - Name: got.Name + "-node-ssh", - ServerID: got.Status.Bastion.ServerID, - BastionSecurityGroupID: got.Status.Bastion.SecurityGroupID, - Tags: bastionservice.NodeSSHAccessTags(got), - }) - Expect(err).NotTo(HaveOccurred()) Expect(fakeCloud.SecurityGroupCount()).To(Equal(2)) configMap := &corev1.ConfigMap{} diff --git a/controller/stackitcluster_infrastructure.go b/controller/stackitcluster_infrastructure.go index f2c55e8..8bbaf7a 100644 --- a/controller/stackitcluster_infrastructure.go +++ b/controller/stackitcluster_infrastructure.go @@ -34,6 +34,7 @@ import ( bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" loadbalancerservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/loadbalancer" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/scope" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -385,8 +386,9 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS } // Cleanup runs unconditionally: neither spec nor status is a trustworthy - // record of what exists in the cloud. ResolveID, DeleteBastion and - // DeleteNodeSSHAccess fall back to tag lookups and tolerate NotFound. + // record of what exists in the cloud. ResolveID, DeleteBastion, and the + // public IP and security group services fall back to tag lookups and + // tolerate NotFound. cloudClient, err := util.BuildCloudClient(ctx, r.Client, r.CloudClientFactory, stackitCluster) if err != nil { // A missing credentials Secret cannot be recovered from and commonly @@ -435,12 +437,6 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS "Deleted API server load balancer %s", loadBalancerID, ) } - if err := cloudClient.DeleteNodeSSHAccess(ctx, bastionservice.NodeSSHAccessTags(stackitCluster)); err != nil && !cloud.IsNotFound(err) { - log.Error(err, "Failed to delete bastion node SSH access", "cluster", stackitCluster.Name) - record.Warnf(stackitCluster, "FailedDeleteNodeSSHAccess", - "Failed to delete bastion node SSH access: %v", err) - return ctrl.Result{}, err - } hadBastion := hasBastionStatus(stackitCluster.Status.Bastion) if err := publicip.New(clusterScope).Delete(ctx); err != nil && !cloud.IsNotFound(err) { log.Error(err, "Failed to delete bastion public IP", "cluster", stackitCluster.Name) @@ -448,6 +444,7 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS "Failed to delete bastion public IP: %v", err) return ctrl.Result{}, err } + if err := cloudClient.DeleteBastion(ctx, bastionservice.Input(stackitCluster, nil), cloud.Bastion{ ServerID: stackitCluster.Status.Bastion.ServerID, SecurityGroupID: stackitCluster.Status.Bastion.SecurityGroupID, @@ -457,6 +454,12 @@ func (r *StackitClusterReconciler) reconcileDelete(ctx context.Context, clusterS "Failed to delete bastion: %v", err) return ctrl.Result{}, err } + if err := securitygroup.New(clusterScope).Delete(ctx); err != nil && !cloud.IsNotFound(err) { + log.Error(err, "Failed to delete bastion security groups", "cluster", stackitCluster.Name) + record.Warnf(stackitCluster, "FailedDeleteBastion", + "Failed to delete bastion security groups: %v", err) + return ctrl.Result{}, err + } if hadBastion { clusterScope.ClearBastionStatus() record.Event(stackitCluster, "BastionDeleted", "Deleted bastion") diff --git a/controller/stackitmachine_controller_test.go b/controller/stackitmachine_controller_test.go index 0efd701..85b1b39 100644 --- a/controller/stackitmachine_controller_test.go +++ b/controller/stackitmachine_controller_test.go @@ -28,6 +28,7 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" cloudfake "github.com/stackitcloud/cluster-api-provider-stackit/cloud/fake" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -237,7 +238,7 @@ var _ = Describe("StackitMachine Controller", func() { got := &infrav1.StackitMachine{} Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed()) Expect(got.Status.Ready).To(BeTrue()) - Expect(fakeCloud.EnsureNodeSSHCalls).To(Equal(1)) + Expect(fakeCloud.AddSecurityGroupCalls).To(Equal(1)) securityGroups, err := fakeCloud.ListSecurityGroupsByTags(ctx, map[string]string{ util.LabelClusterName: clusterName, util.LabelResourceRole: util.ResourceRoleNodeSSH, @@ -245,6 +246,18 @@ var _ = Describe("StackitMachine Controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(securityGroups).To(HaveLen(1)) Expect(fakeCloud.SecurityGroupRemoteSources(securityGroups[0].ID)).To(ConsistOf("bastion-sg")) + Expect(fakeCloud.SecurityGroupRules(securityGroups[0].ID)).To(ConsistOf( + securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: 22, + PortMax: 22, + RemoteSecurityGroupID: "bastion-sg", + }, + securitygroup.Rule{ + Direction: securitygroup.DirectionEgress, + }, + )) Expect(fakeCloud.ServerHasSecurityGroup(got.Status.InstanceID, securityGroups[0].ID)).To(BeTrue()) }) @@ -257,7 +270,7 @@ var _ = Describe("StackitMachine Controller", func() { stackitCluster.Status.Bastion.SecurityGroupID = "bastion-sg" Expect(k8sClient.Status().Update(ctx, stackitCluster)).To(Succeed()) - fakeCloud.FailNextEnsureNodeSSH = fmt.Errorf("node ssh access timeout: %w", cloud.ErrTransient) + fakeCloud.FailNextAddSecurityGroup = fmt.Errorf("node ssh access timeout: %w", cloud.ErrTransient) result, err := reconciler.Reconcile(ctx, request) Expect(err).NotTo(HaveOccurred()) diff --git a/controller/stackitmachine_infrastructure.go b/controller/stackitmachine_infrastructure.go index 920f016..ba1ae6a 100644 --- a/controller/stackitmachine_infrastructure.go +++ b/controller/stackitmachine_infrastructure.go @@ -29,7 +29,7 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" - bastionservice "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/bastion" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/scope" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -91,6 +91,7 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS infrav1.MachineReadyCondition, ) } + machineScope.SetCloudClient(cloudClient) machineScope.SetConditions(metav1.ConditionTrue, "Available", "", infrav1.MachineCredentialsReadyCondition) server, created, err := r.ensureServer(ctx, cloudClient, machineScope, bootstrapData) @@ -160,7 +161,7 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS return ctrl.Result{RequeueAfter: 15 * time.Second}, nil } - if err := r.reconcileBastionNodeSSHAccess(ctx, cloudClient, machineScope, server); err != nil { + if err := r.reconcileBastionNodeSSHAccess(ctx, machineScope, server); err != nil { log.Error(err, "Failed to configure bastion SSH access for node", "machine", stackitMachine.Name) record.Warnf( stackitMachine, "FailedNodeSSHAccess", @@ -364,7 +365,6 @@ func (r *StackitMachineReconciler) ensureServer( func (r *StackitMachineReconciler) reconcileBastionNodeSSHAccess( ctx context.Context, - cloudClient cloud.Client, machineScope *scope.MachineScope, server *cloud.Server, ) error { @@ -377,13 +377,7 @@ func (r *StackitMachineReconciler) reconcileBastionNodeSSHAccess( if server == nil || server.ID == "" { return fmt.Errorf("%w: server ID is empty", cloud.ErrTransient) } - _, err := cloudClient.EnsureNodeSSHAccess(ctx, cloud.NodeSSHAccessInput{ - Name: machineScope.StackitCluster.Name + "-node-ssh", - ServerID: server.ID, - BastionSecurityGroupID: machineScope.StackitCluster.Status.Bastion.SecurityGroupID, - Tags: bastionservice.NodeSSHAccessTags(machineScope.StackitCluster), - }) - return err + return securitygroup.New(machineScope).Reconcile(ctx) } func (r *StackitMachineReconciler) getStackitCluster(ctx context.Context, cluster *clusterv1.Cluster) (*infrav1.StackitCluster, error) { diff --git a/scope/cluster_scope.go b/scope/cluster_scope.go index 0bb4c8b..af7c1bc 100644 --- a/scope/cluster_scope.go +++ b/scope/cluster_scope.go @@ -25,10 +25,13 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/util" iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) +const bastionSSHPort = 22 + // ClusterScope holds the per-reconcile state for a StackitCluster. type ClusterScope struct { Client client.Client @@ -91,10 +94,16 @@ func (s *ClusterScope) SetAPIServerEndpoint(endpoint clusterv1.APIEndpoint) { func (s *ClusterScope) SetBastionStatus(bastion *cloud.Bastion, cloudInitHash string) { s.StackitCluster.Status.Bastion.ServerID = bastion.ServerID - s.StackitCluster.Status.Bastion.SecurityGroupID = bastion.SecurityGroupID + if bastion.SecurityGroupID != "" { + s.StackitCluster.Status.Bastion.SecurityGroupID = bastion.SecurityGroupID + } s.StackitCluster.Status.Bastion.CloudInitHash = cloudInitHash } +func (s *ClusterScope) SetBastionSecurityGroupID(id string) { + s.StackitCluster.Status.Bastion.SecurityGroupID = id +} + func (s *ClusterScope) ClearBastionStatus() { s.StackitCluster.Status.Bastion = infrav1.StackitBastionStatus{} } @@ -160,12 +169,78 @@ func (s *ClusterScope) BastionStatus() infrav1.StackitBastionStatus { } func (s *ClusterScope) BastionTags() map[string]string { - tags := util.ClusterTags(s.StackitCluster.Name, s.StackitCluster.Namespace, s.StackitCluster.Spec.AdditionalLabels) + return bastionTags(s.StackitCluster) +} + +func bastionTags(stackitCluster *infrav1.StackitCluster) map[string]string { + tags := util.ClusterTags(stackitCluster.Name, stackitCluster.Namespace, stackitCluster.Spec.AdditionalLabels) tags[util.LabelResourceRole] = util.ResourceRoleBastion return tags } +func NodeSSHTags(stackitCluster *infrav1.StackitCluster) map[string]string { + tags := util.ClusterTags(stackitCluster.Name, stackitCluster.Namespace, stackitCluster.Spec.AdditionalLabels) + tags[util.LabelResourceRole] = util.ResourceRoleNodeSSH + return tags +} + func (s *ClusterScope) SetBastionPublicIP(id, ip string) { s.StackitCluster.Status.Bastion.PublicIPID = id s.StackitCluster.Status.Bastion.PublicIP = ip } + +func (s *ClusterScope) SecurityGroupSpecs() []securitygroup.SecurityGroupSpec { + rules := make([]securitygroup.Rule, 0, len(s.StackitCluster.Spec.Bastion.AllowedCIDRs)+1) + for _, cidr := range s.StackitCluster.Spec.Bastion.AllowedCIDRs { + if cidr == "" { + continue + } + rules = append(rules, securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: bastionSSHPort, + PortMax: bastionSSHPort, + IPRange: cidr, + }) + } + rules = append(rules, securitygroup.Rule{ + Direction: securitygroup.DirectionEgress, + }) + return []securitygroup.SecurityGroupSpec{ + { + ID: s.StackitCluster.Status.Bastion.SecurityGroupID, + Name: s.StackitCluster.Name + "-bastion-ssh", + Description: "Cluster API Provider STACKIT bastion SSH access", + Tags: s.BastionTags(), + Rules: rules, + SetID: s.SetBastionSecurityGroupID, + }, + nodeSSHSecurityGroupSpec(s.StackitCluster, ""), + } +} + +func nodeSSHSecurityGroupSpec(stackitCluster *infrav1.StackitCluster, remoteSecurityGroupID string, serverIDs ...string) securitygroup.SecurityGroupSpec { + ingressRule := securitygroup.Rule{ + Direction: securitygroup.DirectionIngress, + Protocol: securitygroup.ProtocolTCP, + PortMin: bastionSSHPort, + PortMax: bastionSSHPort, + RemoteSecurityGroupID: remoteSecurityGroupID, + } + if remoteSecurityGroupID == "" { + ingressRule.RemoteSecurityGroupTags = bastionTags(stackitCluster) + } + return securitygroup.SecurityGroupSpec{ + Name: stackitCluster.Name + "-node-ssh", + Description: "Cluster API Provider STACKIT node SSH access from bastion", + Tags: NodeSSHTags(stackitCluster), + ServerIDs: serverIDs, + ServerSelector: util.ClusterTags(stackitCluster.Name, stackitCluster.Namespace, nil), + Rules: []securitygroup.Rule{ + ingressRule, + { + Direction: securitygroup.DirectionEgress, + }, + }, + } +} diff --git a/scope/machine_scope.go b/scope/machine_scope.go index b736ce1..aea2b32 100644 --- a/scope/machine_scope.go +++ b/scope/machine_scope.go @@ -21,7 +21,9 @@ import ( infrav1 "github.com/stackitcloud/cluster-api-provider-stackit/api/v1alpha1" "github.com/stackitcloud/cluster-api-provider-stackit/cloud" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/util" + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) // MachineScope holds the per-reconcile state for a StackitMachine. @@ -31,6 +33,7 @@ type MachineScope struct { Machine *clusterv1.Machine StackitCluster *infrav1.StackitCluster StackitMachine *infrav1.StackitMachine + CloudClient cloud.Client patchHelper *patch.Helper } @@ -129,3 +132,35 @@ func (s *MachineScope) AvailabilityZone() string { return DefaultAvailabilityZone(s.StackitCluster) } + +func (s *MachineScope) SetCloudClient(cloudClient cloud.Client) { + s.CloudClient = cloudClient +} + +func (s *MachineScope) ProjectID() string { + if s.StackitCluster.Spec.ProjectID != "" { + return s.StackitCluster.Spec.ProjectID + } + return s.CloudClient.ProjectID() +} + +func (s *MachineScope) Region() string { + if s.StackitCluster.Spec.Region != "" { + return s.StackitCluster.Spec.Region + } + return s.CloudClient.Region() +} + +func (s *MachineScope) IaaSClient() *iaas.APIClient { + return s.CloudClient.IaaSClient() +} + +func (s *MachineScope) SecurityGroupSpecs() []securitygroup.SecurityGroupSpec { + return []securitygroup.SecurityGroupSpec{ + nodeSSHSecurityGroupSpec( + s.StackitCluster, + s.StackitCluster.Status.Bastion.SecurityGroupID, + s.StackitMachine.Status.InstanceID, + ), + } +} diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index d753b39..ebef540 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -35,6 +35,7 @@ import ( "github.com/stackitcloud/cluster-api-provider-stackit/cloud" "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/publicip" + "github.com/stackitcloud/cluster-api-provider-stackit/cloud/services/securitygroup" "github.com/stackitcloud/cluster-api-provider-stackit/test/utils" "github.com/stackitcloud/cluster-api-provider-stackit/util" ) @@ -494,7 +495,7 @@ var _ = Describe("Manager", Ordered, func() { publicIPs, err := publicip.ListByTags(ctx, cloudClient.IaaSClient(), cloudClient.ProjectID(), cloudClient.Region(), stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(publicIPs).To(HaveLen(1)) - securityGroups, err := cloudClient.ListSecurityGroupsByTags(ctx, stackitE2EBastionTags(testID)) + securityGroups, err := securitygroup.NewClient(cloudClient).ListByTags(ctx, stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(securityGroups).To(HaveLen(1)) }, 10*time.Minute, 15*time.Second).Should(Succeed()) @@ -516,7 +517,7 @@ var _ = Describe("Manager", Ordered, func() { if cfg.BastionEnabled { By("verifying the node SSH security group exists") Eventually(func(g Gomega) { - securityGroups, err := cloudClient.ListSecurityGroupsByTags(ctx, stackitE2ENodeSSHTags(testID)) + securityGroups, err := securitygroup.NewClient(cloudClient).ListByTags(ctx, stackitE2ENodeSSHTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(securityGroups).To(HaveLen(1)) }, 10*time.Minute, 15*time.Second).Should(Succeed()) @@ -560,10 +561,11 @@ var _ = Describe("Manager", Ordered, func() { publicIPs, err := publicip.ListByTags(ctx, cloudClient.IaaSClient(), cloudClient.ProjectID(), cloudClient.Region(), stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(publicIPs).To(BeEmpty()) - securityGroups, err := cloudClient.ListSecurityGroupsByTags(ctx, stackitE2EBastionTags(testID)) + sgClient := securitygroup.NewClient(cloudClient) + securityGroups, err := sgClient.ListByTags(ctx, stackitE2EBastionTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(securityGroups).To(BeEmpty()) - securityGroups, err = cloudClient.ListSecurityGroupsByTags(ctx, stackitE2ENodeSSHTags(testID)) + securityGroups, err = sgClient.ListByTags(ctx, stackitE2ENodeSSHTags(testID)) g.Expect(err).NotTo(HaveOccurred()) g.Expect(securityGroups).To(BeEmpty()) }, 20*time.Minute, 15*time.Second).Should(Succeed()) diff --git a/test/utils/utils.go b/test/utils/utils.go index a408630..9600e4b 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -35,10 +35,12 @@ const ( defaultKindCluster = "kind" ) +// TODO: unreachable func warnError(err error) { _, _ = fmt.Fprintf(GinkgoWriter, "warning: %v\n", err) } +// TODO: unreachable // Run executes the provided command within this context func Run(cmd *exec.Cmd) (string, error) { dir, _ := GetProjectDir() @@ -59,6 +61,7 @@ func Run(cmd *exec.Cmd) (string, error) { return string(output), nil } +// TODO: unreachable // UninstallCertManager uninstalls the cert manager func UninstallCertManager() { url := fmt.Sprintf(certmanagerURLTmpl, certmanagerVersion) @@ -81,6 +84,7 @@ func UninstallCertManager() { } } +// TODO: unreachable // InstallCertManager installs the cert manager bundle. func InstallCertManager() error { url := fmt.Sprintf(certmanagerURLTmpl, certmanagerVersion) @@ -100,6 +104,7 @@ func InstallCertManager() error { return err } +// TODO: unreachable // IsCertManagerCRDsInstalled checks if any Cert Manager CRDs are installed // by verifying the existence of key CRDs related to Cert Manager. func IsCertManagerCRDsInstalled() bool { @@ -133,6 +138,7 @@ func IsCertManagerCRDsInstalled() bool { return false } +// TODO: unreachable // LoadImageToKindClusterWithName loads a local docker image to the kind cluster func LoadImageToKindClusterWithName(name string) error { cluster := defaultKindCluster @@ -149,6 +155,7 @@ func LoadImageToKindClusterWithName(name string) error { return err } +// TODO: unreachable // GetNonEmptyLines converts given command output string into individual objects // according to line breakers, and ignores the empty elements in it. func GetNonEmptyLines(output string) []string { @@ -163,6 +170,7 @@ func GetNonEmptyLines(output string) []string { return res } +// TODO: unreachable // GetProjectDir will return the directory where the project is func GetProjectDir() (string, error) { wd, err := os.Getwd() @@ -173,6 +181,7 @@ func GetProjectDir() (string, error) { return wd, nil } +// TODO: unreachable // UncommentCode searches for target in the file and remove the comment prefix // of the target content. The target content may span multiple lines. func UncommentCode(filename, target, prefix string) error {