Skip to content

Commit dce0af3

Browse files
EmilienMk8s-infra-cherrypick-robot
authored andcommitted
allNodesSecurityGroupRules: relax remote fields
Don't make `remoteManagedGroups` required, since a user can use `remoteIPPrefix` instead or even no remote parameter at all. It's fine because Neutron will set it to an fully-open CIDR if no remote field is provided.
1 parent 28e10fd commit dce0af3

File tree

4 files changed

+74
-15
lines changed

4 files changed

+74
-15
lines changed

docs/book/src/clusteropenstack/configuration.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,7 @@ permitted from anywhere, as with the default rules).
565565
We can add security group rules that authorize traffic from all nodes via `allNodesSecurityGroupRules`.
566566
It takes a list of security groups rules that should be applied to selected nodes.
567567
The following rule fields are mutually exclusive: `remoteManagedGroups`, `remoteGroupID` and `remoteIPPrefix`.
568+
If none of these fields are set, the rule will have a remote IP prefix of `0.0.0.0/0` per Neutron default.
568569

569570
Valid values for `remoteManagedGroups` are `controlplane`, `worker` and `bastion`.
570571

docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,9 @@ The field `managedSecurityGroups` is now a pointer to a `ManagedSecurityGroups`
396396
Also, we can now add security group rules that authorize traffic from all nodes via `allNodesSecurityGroupRules`.
397397
It takes a list of security groups rules that should be applied to selected nodes.
398398
The following rule fields are mutually exclusive: `remoteManagedGroups`, `remoteGroupID` and `remoteIPPrefix`.
399+
If none of these fields are set, the rule will have a remote IP prefix of `0.0.0.0/0` per Neutron default.
400+
401+
```yaml
399402
Valid values for `remoteManagedGroups` are `controlplane`, `worker` and `bastion`.
400403

401404
Also, `OpenStackCluster.Spec.AllowAllInClusterTraffic` moved under `ManagedSecurityGroups`.

pkg/cloud/services/networking/securitygroups.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,6 @@ func getAllNodesRules(remoteManagedGroups map[string]string, allNodesSecurityGro
297297

298298
// validateRemoteManagedGroups validates that the remoteManagedGroups target existing managed security groups.
299299
func validateRemoteManagedGroups(remoteManagedGroups map[string]string, ruleRemoteManagedGroups []infrav1.ManagedSecurityGroupName) error {
300-
if len(ruleRemoteManagedGroups) == 0 {
301-
return fmt.Errorf("remoteManagedGroups is required")
302-
}
303-
304300
for _, group := range ruleRemoteManagedGroups {
305301
if _, ok := remoteManagedGroups[group.String()]; !ok {
306302
return fmt.Errorf("remoteManagedGroups: %s is not a valid remote managed security group", group)

pkg/cloud/services/networking/securitygroups_test.go

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,14 @@ func TestValidateRemoteManagedGroups(t *testing.T) {
4949
wantErr: true,
5050
},
5151
{
52-
name: "Valid rule with missing remoteManagedGroups",
52+
name: "Valid rule with no remoteManagedGroups",
5353
rule: infrav1.SecurityGroupRuleSpec{
54-
PortRangeMin: ptr.To(22),
55-
PortRangeMax: ptr.To(22),
56-
Protocol: ptr.To("tcp"),
54+
PortRangeMin: ptr.To(22),
55+
PortRangeMax: ptr.To(22),
56+
Protocol: ptr.To("tcp"),
57+
RemoteIPPrefix: ptr.To("0.0.0.0/0"),
5758
},
58-
remoteManagedGroups: map[string]string{
59-
"self": "self",
60-
"controlplane": "1",
61-
"worker": "2",
62-
"bastion": "3",
63-
},
64-
wantErr: true,
59+
wantErr: false,
6560
},
6661
{
6762
name: "Valid rule with remoteManagedGroups",
@@ -171,6 +166,70 @@ func TestGetAllNodesRules(t *testing.T) {
171166
},
172167
},
173168
},
169+
{
170+
name: "Valid remoteIPPrefix in a rule",
171+
remoteManagedGroups: map[string]string{
172+
"controlplane": "1",
173+
"worker": "2",
174+
},
175+
allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{
176+
{
177+
Protocol: ptr.To("tcp"),
178+
PortRangeMin: ptr.To(22),
179+
PortRangeMax: ptr.To(22),
180+
RemoteIPPrefix: ptr.To("0.0.0.0/0"),
181+
},
182+
},
183+
wantRules: []resolvedSecurityGroupRuleSpec{
184+
{
185+
Protocol: "tcp",
186+
PortRangeMin: 22,
187+
PortRangeMax: 22,
188+
RemoteIPPrefix: "0.0.0.0/0",
189+
},
190+
},
191+
},
192+
{
193+
name: "Valid allNodesSecurityGroupRules with no remote parameter",
194+
remoteManagedGroups: map[string]string{
195+
"controlplane": "1",
196+
"worker": "2",
197+
},
198+
allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{
199+
{
200+
Protocol: ptr.To("tcp"),
201+
PortRangeMin: ptr.To(22),
202+
PortRangeMax: ptr.To(22),
203+
},
204+
},
205+
wantRules: []resolvedSecurityGroupRuleSpec{
206+
{
207+
Protocol: "tcp",
208+
PortRangeMin: 22,
209+
PortRangeMax: 22,
210+
},
211+
},
212+
wantErr: false,
213+
},
214+
{
215+
name: "Invalid allNodesSecurityGroupRules with bastion while remoteManagedGroups does not have bastion",
216+
remoteManagedGroups: map[string]string{
217+
"controlplane": "1",
218+
"worker": "2",
219+
},
220+
allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{
221+
{
222+
Protocol: ptr.To("tcp"),
223+
PortRangeMin: ptr.To(22),
224+
PortRangeMax: ptr.To(22),
225+
RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{
226+
"bastion",
227+
},
228+
},
229+
},
230+
wantRules: nil,
231+
wantErr: true,
232+
},
174233
{
175234
name: "Invalid allNodesSecurityGroupRules with wrong remoteManagedGroups",
176235
remoteManagedGroups: map[string]string{

0 commit comments

Comments
 (0)