Skip to content
This repository was archived by the owner on Oct 22, 2024. It is now read-only.

Commit b206885

Browse files
mjlshenmjlshen
committed
Validate that EC2 instances only have one security group attached
Signed-off-by: Michael Shen <mshen@redhat.com>
1 parent c027549 commit b206885

File tree

2 files changed

+182
-13
lines changed

2 files changed

+182
-13
lines changed

‎pkg/mirrosa/instances.go‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,22 @@ import (
1111
"go.uber.org/zap"
1212
)
1313

14-
const instanceDescription = `A ROSA cluster must have the followings
15-
- 3 masters running
16-
- at least 2 infras running for single-AZ, 3 infras for multi-AZ`
14+
const instanceDescription = `A ROSA cluster must have the following:
15+
- 3 control plane instances running
16+
- at least 2 infra instances running for single-AZ, 3 infra instances for multi-AZ`
1717

1818
var _ Component = &Instances{}
1919

20+
type MirrosaInstancesAPIClient interface {
21+
ec2.DescribeInstancesAPIClient
22+
}
23+
2024
type Instances struct {
2125
log *zap.SugaredLogger
2226
InfraName string
2327
MultiAZ bool
2428

25-
Ec2Client Ec2AwsApi
29+
Ec2Client MirrosaInstancesAPIClient
2630
}
2731

2832
func (c *Client) NewInstances() Instances {
@@ -61,7 +65,7 @@ func (i Instances) Validate(ctx context.Context) error {
6165
}
6266

6367
// MASTER NODES VALIDATIONS
64-
i.log.Info("validating cluster's master nodes")
68+
i.log.Info("validating cluster's control plane instances")
6569
var masters []types.Instance
6670
masterPattern := fmt.Sprintf("%s-master", i.InfraName)
6771
for _, v := range instances {
@@ -74,18 +78,24 @@ func (i Instances) Validate(ctx context.Context) error {
7478

7579
// Each cluster has 3 master nodes by default - immutable
7680
if len(masters) != 3 {
77-
return fmt.Errorf("there should be 3 masters belong to the cluster")
81+
return fmt.Errorf("there should be 3 control plane instances, found %d", len(masters))
7882
}
7983

8084
// Check if masters are running
8185
for _, v := range masters {
8286
if v.State.Name != types.InstanceStateNameRunning {
83-
return fmt.Errorf("found non running master instance: %s", *v.InstanceId)
87+
return fmt.Errorf("found non running control plane instance: %s", *v.InstanceId)
88+
}
89+
90+
if len(v.SecurityGroups) != 1 {
91+
return fmt.Errorf("one security group should be attached to %s: (%s-master-sg), got %d", *v.InstanceId, i.InfraName, len(v.SecurityGroups))
8492
}
93+
94+
// TODO: Check if the security group is the correct one, with tag "Name: ${infra_name}-master-sg"
8595
}
8696

8797
// INFRA NODES VALIDATIONS
88-
i.log.Info("validating cluster's infra nodes")
98+
i.log.Info("validating cluster's infra instances")
8999
var infraNodes []types.Instance
90100
infraPattern := fmt.Sprintf("%s-infra", i.InfraName)
91101
for _, v := range instances {
@@ -97,22 +107,28 @@ func (i Instances) Validate(ctx context.Context) error {
97107
}
98108

99109
if i.MultiAZ && len(infraNodes) < 3 {
100-
return fmt.Errorf("there should be at least 3 infra nodes for multi-AZ clusters")
110+
return fmt.Errorf("there should be at least 3 infra instances for multi-AZ clusters")
101111
}
102112

103113
if !i.MultiAZ && len(infraNodes) < 2 {
104-
return fmt.Errorf("there should be at least 2 infra nodes for single-AZ clusters")
114+
return fmt.Errorf("there should be at least 2 infra instances for single-AZ clusters")
105115
}
106116

107117
// Check if infras are running
108118
for _, v := range infraNodes {
109119
if v.State.Name != types.InstanceStateNameRunning {
110-
return fmt.Errorf("found non running infra node: %s", *v.InstanceId)
120+
return fmt.Errorf("found non running infra instances: %s", *v.InstanceId)
111121
}
122+
123+
if len(v.SecurityGroups) != 1 {
124+
return fmt.Errorf("one security group should be attached to %s: (%s-worker-sg), got %d", *v.InstanceId, i.InfraName, len(v.SecurityGroups))
125+
}
126+
127+
// TODO: Check if the security group is the correct one, with tag "Name: ${infra_name}-worker-sg"
112128
}
113129

114130
// WORKER NODES VALIDATIONS
115-
i.log.Info("validating cluster's worker nodes")
131+
i.log.Info("validating cluster's worker instances")
116132
var workerNodes []types.Instance
117133
workerPattern := fmt.Sprintf("%s-worker", i.InfraName)
118134
for _, v := range instances {
@@ -133,6 +149,12 @@ func (i Instances) Validate(ctx context.Context) error {
133149
if v.State.Name != types.InstanceStateNameRunning {
134150
i.log.Infof("[error but not blocker]: found non running worker nodes: %s", *v.InstanceId)
135151
}
152+
153+
if len(v.SecurityGroups) != 1 {
154+
return fmt.Errorf("one security group should be attached to %s: (%s-worker-sg), got %d", *v.InstanceId, i.InfraName, len(v.SecurityGroups))
155+
}
156+
157+
// TODO: Check if the security group is the correct one, with tag "Name: ${infra_name}-worker-sg"
136158
}
137159

138160
return nil
@@ -143,5 +165,5 @@ func (i Instances) Documentation() string {
143165
}
144166

145167
func (i Instances) FilterValue() string {
146-
return "instance validation service"
168+
return "EC2 Instance"
147169
}

‎pkg/mirrosa/instances_test.go‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
package mirrosa
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/aws/aws-sdk-go-v2/aws"
8+
"github.com/aws/aws-sdk-go-v2/service/ec2"
9+
"github.com/aws/aws-sdk-go-v2/service/ec2/types"
10+
"go.uber.org/zap/zaptest"
11+
)
12+
13+
type mockMirrosaInstancesAPIClient struct {
14+
describeInstancesResp *ec2.DescribeInstancesOutput
15+
}
16+
17+
func (m mockMirrosaInstancesAPIClient) DescribeInstances(ctx context.Context, params *ec2.DescribeInstancesInput, optFns ...func(options *ec2.Options)) (*ec2.DescribeInstancesOutput, error) {
18+
return m.describeInstancesResp, nil
19+
}
20+
21+
func TestInstances_Validate(t *testing.T) {
22+
tests := []struct {
23+
name string
24+
instances *Instances
25+
expectErr bool
26+
}{
27+
{
28+
name: "no instances",
29+
instances: &Instances{
30+
log: zaptest.NewLogger(t).Sugar(),
31+
Ec2Client: &mockMirrosaInstancesAPIClient{
32+
describeInstancesResp: &ec2.DescribeInstancesOutput{
33+
Reservations: []types.Reservation{},
34+
},
35+
},
36+
},
37+
expectErr: true,
38+
},
39+
{
40+
name: "healthy multi-az",
41+
instances: &Instances{
42+
log: zaptest.NewLogger(t).Sugar(),
43+
InfraName: "mock",
44+
MultiAZ: true,
45+
Ec2Client: &mockMirrosaInstancesAPIClient{
46+
describeInstancesResp: &ec2.DescribeInstancesOutput{
47+
Reservations: []types.Reservation{
48+
{
49+
Groups: nil,
50+
Instances: []types.Instance{
51+
{
52+
SecurityGroups: []types.GroupIdentifier{
53+
{},
54+
},
55+
State: &types.InstanceState{Name: types.InstanceStateNameRunning},
56+
Tags: []types.Tag{
57+
{Key: aws.String("Name"), Value: aws.String("mock-master1")},
58+
},
59+
},
60+
{
61+
SecurityGroups: []types.GroupIdentifier{
62+
{},
63+
},
64+
State: &types.InstanceState{Name: types.InstanceStateNameRunning},
65+
Tags: []types.Tag{
66+
{Key: aws.String("Name"), Value: aws.String("mock-master2")},
67+
},
68+
},
69+
{
70+
SecurityGroups: []types.GroupIdentifier{
71+
{},
72+
},
73+
State: &types.InstanceState{Name: types.InstanceStateNameRunning},
74+
Tags: []types.Tag{
75+
{Key: aws.String("Name"), Value: aws.String("mock-master3")},
76+
},
77+
},
78+
{
79+
SecurityGroups: []types.GroupIdentifier{
80+
{},
81+
},
82+
State: &types.InstanceState{Name: types.InstanceStateNameRunning},
83+
Tags: []types.Tag{
84+
{Key: aws.String("Name"), Value: aws.String("mock-infra1")},
85+
},
86+
},
87+
{
88+
SecurityGroups: []types.GroupIdentifier{
89+
{},
90+
},
91+
State: &types.InstanceState{Name: types.InstanceStateNameRunning},
92+
Tags: []types.Tag{
93+
{Key: aws.String("Name"), Value: aws.String("mock-infra2")},
94+
},
95+
},
96+
{
97+
SecurityGroups: []types.GroupIdentifier{
98+
{},
99+
},
100+
State: &types.InstanceState{Name: types.InstanceStateNameRunning},
101+
Tags: []types.Tag{
102+
{Key: aws.String("Name"), Value: aws.String("mock-infra3")},
103+
},
104+
},
105+
{
106+
SecurityGroups: []types.GroupIdentifier{
107+
{},
108+
},
109+
State: &types.InstanceState{Name: types.InstanceStateNameRunning},
110+
Tags: []types.Tag{
111+
{Key: aws.String("Name"), Value: aws.String("mock-worker1")},
112+
},
113+
},
114+
{
115+
SecurityGroups: []types.GroupIdentifier{
116+
{},
117+
},
118+
State: &types.InstanceState{Name: types.InstanceStateNameRunning},
119+
Tags: []types.Tag{
120+
{Key: aws.String("Name"), Value: aws.String("mock-worker2")},
121+
},
122+
},
123+
},
124+
},
125+
},
126+
},
127+
},
128+
},
129+
expectErr: false,
130+
},
131+
}
132+
133+
for _, test := range tests {
134+
t.Run(test.name, func(t *testing.T) {
135+
err := test.instances.Validate(context.TODO())
136+
if err != nil {
137+
if !test.expectErr {
138+
t.Errorf("expected no err, got %v", err)
139+
}
140+
} else {
141+
if test.expectErr {
142+
t.Error("expected err, got nil")
143+
}
144+
}
145+
})
146+
}
147+
}

0 commit comments

Comments
 (0)