Merge pull request kubernetes-sigs#555 from hidekazuna/fix_secgroup

🐛 Fix auto generate security groups
pramchan · Jun 15, 2020 · e5037a9 · e5037a9
2 parents 4343958 + e300493
commit e5037a9
Show file tree

Hide file tree

Showing 8 changed files with 275 additions and 155 deletions.
diff --git a/api/v1alpha3/openstackcluster_types.go b/api/v1alpha3/openstackcluster_types.go
@@ -136,9 +136,9 @@ type OpenStackClusterStatus struct {
 	// TODO: Maybe instead of two properties, we add a property to the group?
 	ControlPlaneSecurityGroup *SecurityGroup `json:"controlPlaneSecurityGroup,omitempty"`
 
-	// GlobalSecurityGroup contains all the information about the OpenStack Security
-	// Group that needs to be applied to all nodes, both control plane and worker nodes.
-	GlobalSecurityGroup *SecurityGroup `json:"globalSecurityGroup,omitempty"`
+	// WorkerSecurityGroup contains all the information about the OpenStack Security
+	// Group that needs to be applied to worker nodes.
+	WorkerSecurityGroup *SecurityGroup `json:"workerSecurityGroup,omitempty"`
 }
 
 // +kubebuilder:object:root=true

diff --git a/api/v1alpha3/types.go b/api/v1alpha3/types.go
@@ -190,6 +190,7 @@ type SecurityGroup struct {
 // SecurityGroupRule represent the basic information of the associated OpenStack
 // Security Group Role.
 type SecurityGroupRule struct {
+	Description     string `json:"description"`
 	ID              string `json:"name"`
 	Direction       string `json:"direction"`
 	EtherType       string `json:"etherType"`
@@ -204,6 +205,7 @@ type SecurityGroupRule struct {
 // Equal checks if two SecurityGroupRules are the same.
 func (r SecurityGroupRule) Equal(x SecurityGroupRule) bool {
 	return (r.Direction == x.Direction &&
+		r.Description == x.Description &&
 		r.EtherType == x.EtherType &&
 		r.PortRangeMin == x.PortRangeMin &&
 		r.PortRangeMax == x.PortRangeMax &&

diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
@@ -373,6 +373,8 @@ spec:
                       description: SecurityGroupRule represent the basic information
                         of the associated OpenStack Security Group Role.
                       properties:
+                        description:
+                          type: string
                         direction:
                           type: string
                         etherType:
@@ -392,6 +394,7 @@ spec:
                         securityGroupID:
                           type: string
                       required:
+                      - description
                       - direction
                       - etherType
                       - name
@@ -427,55 +430,6 @@ spec:
                   type: object
                 description: FailureDomains represent OpenStack availability zones
                 type: object
-              globalSecurityGroup:
-                description: GlobalSecurityGroup contains all the information about
-                  the OpenStack Security Group that needs to be applied to all nodes,
-                  both control plane and worker nodes.
-                properties:
-                  id:
-                    type: string
-                  name:
-                    type: string
-                  rules:
-                    items:
-                      description: SecurityGroupRule represent the basic information
-                        of the associated OpenStack Security Group Role.
-                      properties:
-                        direction:
-                          type: string
-                        etherType:
-                          type: string
-                        name:
-                          type: string
-                        portRangeMax:
-                          type: integer
-                        portRangeMin:
-                          type: integer
-                        protocol:
-                          type: string
-                        remoteGroupID:
-                          type: string
-                        remoteIPPrefix:
-                          type: string
-                        securityGroupID:
-                          type: string
-                      required:
-                      - direction
-                      - etherType
-                      - name
-                      - portRangeMax
-                      - portRangeMin
-                      - protocol
-                      - remoteGroupID
-                      - remoteIPPrefix
-                      - securityGroupID
-                      type: object
-                    type: array
-                required:
-                - id
-                - name
-                - rules
-                type: object
               network:
                 description: Network contains all information about the created OpenStack
                   Network. It includes Subnets and Router.
@@ -535,6 +489,58 @@ spec:
                 type: object
               ready:
                 type: boolean
+              workerSecurityGroup:
+                description: WorkerSecurityGroup contains all the information about
+                  the OpenStack Security Group that needs to be applied to worker
+                  nodes.
+                properties:
+                  id:
+                    type: string
+                  name:
+                    type: string
+                  rules:
+                    items:
+                      description: SecurityGroupRule represent the basic information
+                        of the associated OpenStack Security Group Role.
+                      properties:
+                        description:
+                          type: string
+                        direction:
+                          type: string
+                        etherType:
+                          type: string
+                        name:
+                          type: string
+                        portRangeMax:
+                          type: integer
+                        portRangeMin:
+                          type: integer
+                        protocol:
+                          type: string
+                        remoteGroupID:
+                          type: string
+                        remoteIPPrefix:
+                          type: string
+                        securityGroupID:
+                          type: string
+                      required:
+                      - description
+                      - direction
+                      - etherType
+                      - name
+                      - portRangeMax
+                      - portRangeMin
+                      - protocol
+                      - remoteGroupID
+                      - remoteIPPrefix
+                      - securityGroupID
+                      type: object
+                    type: array
+                required:
+                - id
+                - name
+                - rules
+                type: object
             required:
             - ready
             type: object

diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go
@@ -136,9 +136,9 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, log lo
 	}
 
 	// Delete other things
-	if openStackCluster.Status.GlobalSecurityGroup != nil {
-		log.Info("Deleting global security group", "name", openStackCluster.Status.GlobalSecurityGroup.Name)
-		err := networkingService.DeleteSecurityGroups(openStackCluster.Status.GlobalSecurityGroup)
+	if openStackCluster.Status.WorkerSecurityGroup != nil {
+		log.Info("Deleting worker security group", "name", openStackCluster.Status.WorkerSecurityGroup.Name)
+		err := networkingService.DeleteSecurityGroups(openStackCluster.Status.WorkerSecurityGroup)
 		if err != nil {
 			return reconcile.Result{}, errors.Errorf("failed to delete security group: %v", err)
 		}

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -67,6 +67,21 @@ openstack keypair create <name>
 ```
 The keypair must be exposed as an environment variable `OPENSTACK_SSH_AUTHORIZED_KEY`.
 
+If you want to login to each machine by ssh, you have to configure security groups. If `spec.managedSecurityGroups` of `OpenStackCluster` set to true, two security groups will be created. One is `k8s-cluster-${NAMESPACE}-${CLUSTER_NAME}-secgroup-controlplane`, another is `k8s-cluster-${NAMESPACE}-${CLUSTER_NAME}-secgroup-worker`. These security group rules are following kubeadm's [Check required ports](https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/install-kubeadm/#check-required-ports) so that each node can not be logged in through ssh by default. Please add existing security group allowing ssh port to `OpenStackMachineTemplate` spec. Here is an example:
+
+```yaml
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha3
+kind: OpenStackMachineTemplate
+metadata:
+  name: ${CLUSTER_NAME}-control-plane
+spec:
+  template:
+    spec:
+      securityGroups:
+      - name: k8s-cluster-${NAMESPACE}-${CLUSTER_NAME}-secgroup-controlplane
+      - name: allow-ssh
+```
+
 ## Network Filters
 
 If you have a complex query that you want to use to lookup a network, then you can do this by using a network filter. More details about the filter can be found in [NetworkParam](../api/v1alpha3/types.go)