Skip to content

Commit

Permalink
allNodesSecurityGroupRules: relax remote fields
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EmilienM committed May 14, 2024
1 parent 3489eef commit 392f092
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 15 deletions.
1 change: 1 addition & 0 deletions docs/book/src/clusteropenstack/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@ permitted from anywhere, as with the default rules).
We can add security group rules that authorize traffic from all nodes via `allNodesSecurityGroupRules`.
It takes a list of security groups rules that should be applied to selected nodes.
The following rule fields are mutually exclusive: `remoteManagedGroups`, `remoteGroupID` and `remoteIPPrefix`.
If none of these fields are set, the rule will have a remote IP prefix of `0.0.0.0/0` per Neutron default.

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

Expand Down
3 changes: 3 additions & 0 deletions docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ The field `managedSecurityGroups` is now a pointer to a `ManagedSecurityGroups`
Also, we can now add security group rules that authorize traffic from all nodes via `allNodesSecurityGroupRules`.
It takes a list of security groups rules that should be applied to selected nodes.
The following rule fields are mutually exclusive: `remoteManagedGroups`, `remoteGroupID` and `remoteIPPrefix`.
If none of these fields are set, the rule will have a remote IP prefix of `0.0.0.0/0` per Neutron default.

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

Also, `OpenStackCluster.Spec.AllowAllInClusterTraffic` moved under `ManagedSecurityGroups`.
Expand Down
4 changes: 0 additions & 4 deletions pkg/cloud/services/networking/securitygroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,6 @@ func getAllNodesRules(remoteManagedGroups map[string]string, allNodesSecurityGro

// validateRemoteManagedGroups validates that the remoteManagedGroups target existing managed security groups.
func validateRemoteManagedGroups(remoteManagedGroups map[string]string, ruleRemoteManagedGroups []infrav1.ManagedSecurityGroupName) error {
if len(ruleRemoteManagedGroups) == 0 {
return fmt.Errorf("remoteManagedGroups is required")
}

for _, group := range ruleRemoteManagedGroups {
if _, ok := remoteManagedGroups[group.String()]; !ok {
return fmt.Errorf("remoteManagedGroups: %s is not a valid remote managed security group", group)
Expand Down
81 changes: 70 additions & 11 deletions pkg/cloud/services/networking/securitygroups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,14 @@ func TestValidateRemoteManagedGroups(t *testing.T) {
wantErr: true,
},
{
name: "Valid rule with missing remoteManagedGroups",
name: "Valid rule with no remoteManagedGroups",
rule: infrav1.SecurityGroupRuleSpec{
PortRangeMin: ptr.To(22),
PortRangeMax: ptr.To(22),
Protocol: ptr.To("tcp"),
PortRangeMin: ptr.To(22),
PortRangeMax: ptr.To(22),
Protocol: ptr.To("tcp"),
RemoteIPPrefix: ptr.To("0.0.0.0/0"),
},
remoteManagedGroups: map[string]string{
"self": "self",
"controlplane": "1",
"worker": "2",
"bastion": "3",
},
wantErr: true,
wantErr: false,
},
{
name: "Valid rule with remoteManagedGroups",
Expand Down Expand Up @@ -171,6 +166,70 @@ func TestGetAllNodesRules(t *testing.T) {
},
},
},
{
name: "Valid remoteIPPrefix in a rule",
remoteManagedGroups: map[string]string{
"controlplane": "1",
"worker": "2",
},
allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{
{
Protocol: ptr.To("tcp"),
PortRangeMin: ptr.To(22),
PortRangeMax: ptr.To(22),
RemoteIPPrefix: ptr.To("0.0.0.0/0"),
},
},
wantRules: []resolvedSecurityGroupRuleSpec{
{
Protocol: "tcp",
PortRangeMin: 22,
PortRangeMax: 22,
RemoteIPPrefix: "0.0.0.0/0",
},
},
},
{
name: "Valid allNodesSecurityGroupRules with no remote parameter",
remoteManagedGroups: map[string]string{
"controlplane": "1",
"worker": "2",
},
allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{
{
Protocol: ptr.To("tcp"),
PortRangeMin: ptr.To(22),
PortRangeMax: ptr.To(22),
},
},
wantRules: []resolvedSecurityGroupRuleSpec{
{
Protocol: "tcp",
PortRangeMin: 22,
PortRangeMax: 22,
},
},
wantErr: false,
},
{
name: "Invalid allNodesSecurityGroupRules with bastion while remoteManagedGroups does not have bastion",
remoteManagedGroups: map[string]string{
"controlplane": "1",
"worker": "2",
},
allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{
{
Protocol: ptr.To("tcp"),
PortRangeMin: ptr.To(22),
PortRangeMax: ptr.To(22),
RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{
"bastion",
},
},
},
wantRules: nil,
wantErr: true,
},
{
name: "Invalid allNodesSecurityGroupRules with wrong remoteManagedGroups",
remoteManagedGroups: map[string]string{
Expand Down

0 comments on commit 392f092

Please sign in to comment.