add allowed CIDR blocks to HostedCluster API

openshift · a-dsouza · Apr 11, 2022 · Apr 27, 2022 · May 31, 2022 · Jun 14, 2022
commit e6f7ca597ebc1db117bcf34969d89fbdaab8b2c6
diff --git a/api/v1alpha1/hosted_controlplane.go b/api/v1alpha1/hosted_controlplane.go
@@ -59,6 +59,10 @@ type HostedControlPlaneSpec struct {
 	// inside a worker.
 	// +optional
 	APIAdvertiseAddress *string `json:"apiAdvertiseAddress,omitempty"`
+	// APIAllowedCIDRBlocks is an allow list of CIDR blocks that can access the APIServer
+	// If not specified, traffic is allowed from all addresses.
+	// This depends on underlying support by the cloud provider for Service LoadBalancerSourceRanges
+	APIAllowedCIDRBlocks []CIDRBlock `json:"apiAllowedCIDRBlocks,omitempty"`
 
 	// ControllerAvailabilityPolicy specifies whether to run control plane controllers in HA mode
 	// Defaults to SingleReplica when not set

diff --git a/api/v1alpha1/hostedcluster_types.go b/api/v1alpha1/hostedcluster_types.go
@@ -466,6 +466,9 @@ type ClusterNetworking struct {
 	APIServer *APIServerNetworking `json:"apiServer,omitempty"`
 }
 
+//+kubebuilder:validation:Pattern:=`^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])(\/(3[0-2]|[1-2][0-9]|[0-9]))$`
+type CIDRBlock string
+
 // APIServerNetworking specifies how the APIServer is exposed inside a cluster
 // node.
 type APIServerNetworking struct {
@@ -478,6 +481,11 @@ type APIServerNetworking struct {
 	// pods using host networking cannot listen on this port. If not specified,
 	// 6443 is used.
 	Port *int32 `json:"port,omitempty"`
+
+	// AllowedCIDRBlocks is an allow list of CIDR blocks that can access the APIServer
+	// If not specified, traffic is allowed from all addresses.
+	// This depends on underlying support by the cloud provider for Service LoadBalancerSourceRanges
+	AllowedCIDRBlocks []CIDRBlock `json:"allowedCIDRBlocks,omitempty"`
 }
 
 // NetworkType specifies the SDN provider used for cluster networking.

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedclusters.yaml b/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedclusters.yaml
@@ -352,6 +352,15 @@ spec:
                           with the loopback adapter of each node. If not specified,
                           172.20.0.1 is used.
                         type: string
+                      allowedCIDRBlocks:
+                        description: AllowedCIDRBlocks is an allow list of CIDR blocks
+                          that can access the APIServer If not specified, traffic
+                          is allowed from all addresses. This depends on underlying
+                          support by the cloud provider for Service LoadBalancerSourceRanges
+                        items:
+                          pattern: ^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])(\/(3[0-2]|[1-2][0-9]|[0-9]))$
+                          type: string
+                        type: array
                       port:
                         description: Port is the port at which the APIServer is exposed
                           inside a node. Other pods using host networking cannot listen

diff --git a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedcontrolplanes.yaml b/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedcontrolplanes.yaml
@@ -54,6 +54,15 @@ spec:
                 description: APIAdvertiseAddress is the address at which the APIServer
                   listens inside a worker.
                 type: string
+              apiAllowedCIDRBlocks:
+                description: APIAllowedCIDRBlocks is an allow list of CIDR blocks
+                  that can access the APIServer If not specified, traffic is allowed
+                  from all addresses. This depends on underlying support by the cloud
+                  provider for Service LoadBalancerSourceRanges
+                items:
+                  pattern: ^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])(\/(3[0-2]|[1-2][0-9]|[0-9]))$
+                  type: string
+                type: array
               apiPort:
                 description: APIPort is the port at which the APIServer listens inside
                   a worker

diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
@@ -761,7 +761,7 @@ func (r *HostedControlPlaneReconciler) reconcileAPIServerService(ctx context.Con
 	p := kas.NewKubeAPIServerServiceParams(hcp)
 	apiServerService := manifests.KubeAPIServerService(hcp.Namespace)
 	if _, err := r.CreateOrUpdate(ctx, r.Client, apiServerService, func() error {
-		return kas.ReconcileService(apiServerService, serviceStrategy, p.OwnerReference, p.APIServerPort, util.IsPublicHCP(hcp))
+		return kas.ReconcileService(apiServerService, serviceStrategy, p.OwnerReference, p.APIServerPort, p.AllowedCIDRBlocks, util.IsPublicHCP(hcp))
 	}); err != nil {
 		return fmt.Errorf("failed to reconcile API server service: %w", err)
 	}

diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go
@@ -7,12 +7,14 @@ import (
 	. "github.com/onsi/gomega"
 	hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
 	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/common"
+	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests"
 	"github.com/openshift/hypershift/support/globalconfig"
 	"github.com/openshift/hypershift/support/upsert"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -112,3 +114,116 @@ spec:
 		})
 	}
 }
+
+func TestReconcileAPIServerService(t *testing.T) {
+	targetNamespace := "test"
+	apiPort := int32(1234)
+	hostname := "test.example.com"
+	allowCIDR := []hyperv1.CIDRBlock{"1.2.3.4/24"}
+	allowCIDRString := []string{"1.2.3.4/24"}
+	testsCases := []struct {
+		name             string
+		hcp              *hyperv1.HostedControlPlane
+		expectedServices []*corev1.Service
+	}{
+		{
+			name: "EndpointAccess PublicAndPrivate, ServicePublishingStrategy LoadBalancer, hostname, custom port, and allowed CIDR blocks",
+			hcp: &hyperv1.HostedControlPlane{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: targetNamespace,
+					Name:      "test",
+				},
+				Spec: hyperv1.HostedControlPlaneSpec{
+					APIPort:              &apiPort,
+					APIAllowedCIDRBlocks: allowCIDR,
+					Platform: hyperv1.PlatformSpec{
+						Type: hyperv1.AWSPlatform,
+						AWS: &hyperv1.AWSPlatformSpec{
+							EndpointAccess: hyperv1.PublicAndPrivate,
+						},
+					},
+					Services: []hyperv1.ServicePublishingStrategyMapping{
+						{
+							Service: hyperv1.APIServer,
+							ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{
+								Type: hyperv1.LoadBalancer,
+								LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{
+									Hostname: hostname,
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedServices: []*corev1.Service{
+				{
+					TypeMeta: metav1.TypeMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: targetNamespace,
+						Name:      manifests.KubeAPIServerService(targetNamespace).Name,
+						Annotations: map[string]string{
+							"service.beta.kubernetes.io/aws-load-balancer-type": "nlb",
+							hyperv1.ExternalDNSHostnameAnnotation:               hostname,
+						},
+					},
+					Spec: corev1.ServiceSpec{
+						Type: corev1.ServiceTypeLoadBalancer,
+						Ports: []corev1.ServicePort{
+							{
+								Protocol:   corev1.ProtocolTCP,
+								Port:       apiPort,
+								TargetPort: intstr.FromInt(int(apiPort)),
+							},
+						},
+						LoadBalancerSourceRanges: allowCIDRString,
+					},
+				},
+				{
+					TypeMeta: metav1.TypeMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: targetNamespace,
+						Name:      manifests.KubeAPIServerPrivateService(targetNamespace).Name,
+						Annotations: map[string]string{
+							"service.beta.kubernetes.io/aws-load-balancer-type":     "nlb",
+							"service.beta.kubernetes.io/aws-load-balancer-internal": "true",
+						},
+					},
+					Spec: corev1.ServiceSpec{
+						Type: corev1.ServiceTypeLoadBalancer,
+						Ports: []corev1.ServicePort{
+							{
+								Protocol:   corev1.ProtocolTCP,
+								Port:       6443,
+								TargetPort: intstr.FromInt(6443),
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range testsCases {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewGomegaWithT(t)
+
+			fakeClient := fake.NewClientBuilder().Build()
+			r := &HostedControlPlaneReconciler{
+				Client:                 fakeClient,
+				Log:                    ctrl.LoggerFrom(context.TODO()),
+				CreateOrUpdateProvider: upsert.New(false),
+			}
+
+			err := r.reconcileAPIServerService(context.Background(), tc.hcp)
+			g.Expect(err).NotTo(HaveOccurred())
+			var actualService corev1.Service
+			for _, expectedService := range tc.expectedServices {
+				err = r.Get(context.Background(), client.ObjectKeyFromObject(expectedService), &actualService)
+				g.Expect(err).NotTo(HaveOccurred())
+				actualService.Spec.Selector = nil
+				g.Expect(actualService.Spec).To(Equal(expectedService.Spec))
+				g.Expect(actualService.Annotations).To(Equal(expectedService.Annotations))
+			}
+		})
+	}
+}
diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/params.go b/control-plane-operator/controllers/hostedcontrolplane/kas/params.go
@@ -64,8 +64,9 @@ type KubeAPIServerParams struct {
 }
 
 type KubeAPIServerServiceParams struct {
-	APIServerPort  int
-	OwnerReference *metav1.OwnerReference
+	APIServerPort     int
+	AllowedCIDRBlocks []string
+	OwnerReference    *metav1.OwnerReference
 }
 
 func NewKubeAPIServerParams(ctx context.Context, hcp *hyperv1.HostedControlPlane, globalConfig globalconfig.GlobalConfig, images map[string]string, externalAPIAddress string, externalAPIPort int32, externalOAuthAddress string, externalOAuthPort int32, setDefaultSecurityContext bool) *KubeAPIServerParams {
@@ -478,8 +479,14 @@ func NewKubeAPIServerServiceParams(hcp *hyperv1.HostedControlPlane) *KubeAPIServ
 	if hcp.Spec.APIPort != nil {
 		port = int(*hcp.Spec.APIPort)
 	}
+	var allowedCIDRBlocks []string
+	for _, block := range hcp.Spec.APIAllowedCIDRBlocks {
+		allowedCIDRBlocks = append(allowedCIDRBlocks, string(block))
+	}
+
 	return &KubeAPIServerServiceParams{
-		APIServerPort:  port,
-		OwnerReference: config.ControllerOwnerRef(hcp),
+		APIServerPort:     port,
+		AllowedCIDRBlocks: allowedCIDRBlocks,
+		OwnerReference:    config.ControllerOwnerRef(hcp),
 	}
 }
diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go
@@ -15,7 +15,7 @@ import (
 	"github.com/openshift/hypershift/support/util"
 )
 
-func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingStrategy, owner *metav1.OwnerReference, apiServerPort int, isPublic bool) error {
+func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingStrategy, owner *metav1.OwnerReference, apiServerPort int, apiAllowedCIDRBlocks []string, isPublic bool) error {
 	util.EnsureOwnerRef(svc, owner)
 	if svc.Spec.Selector == nil {
 		svc.Spec.Selector = kasLabels()
@@ -60,6 +60,7 @@ func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingSt
 	default:
 		return fmt.Errorf("invalid publishing strategy for Kube API server service: %s", strategy.Type)
 	}
+	svc.Spec.LoadBalancerSourceRanges = apiAllowedCIDRBlocks
 	svc.Spec.Ports[0] = portSpec
 	return nil
 }

diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md
@@ -761,6 +761,21 @@ pods using host networking cannot listen on this port. If not specified,
 6443 is used.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>allowedCIDRBlocks</code></br>
+<em>
+<a href="#hypershift.openshift.io/v1alpha1.CIDRBlock">
+[]CIDRBlock
+</a>
+</em>
+</td>
+<td>
+<p>AllowedCIDRBlocks is an allow list of CIDR blocks that can access the APIServer
+If not specified, traffic is allowed from all addresses.
+This depends on underlying support by the cloud provider for Service LoadBalancerSourceRanges</p>
+</td>
+</tr>
 </tbody>
 </table>
 ###AWSCloudProviderConfig { #hypershift.openshift.io/v1alpha1.AWSCloudProviderConfig }
@@ -1736,6 +1751,14 @@ string
 </tr>
 </tbody>
 </table>
+###CIDRBlock { #hypershift.openshift.io/v1alpha1.CIDRBlock }
+<p>
+(<em>Appears on:</em>
+<a href="#hypershift.openshift.io/v1alpha1.APIServerNetworking">APIServerNetworking</a>, 
+<a href="#hypershift.openshift.io/v1alpha1.HostedControlPlaneSpec">HostedControlPlaneSpec</a>)
+</p>
+<p>
+</p>
 ###ClusterAutoscaling { #hypershift.openshift.io/v1alpha1.ClusterAutoscaling }
 <p>
 (<em>Appears on:</em>
@@ -2999,6 +3022,21 @@ inside a worker.</p>
 </tr>
 <tr>
 <td>
+<code>apiAllowedCIDRBlocks</code></br>
+<em>
+<a href="#hypershift.openshift.io/v1alpha1.CIDRBlock">
+[]CIDRBlock
+</a>
+</em>
+</td>
+<td>
+<p>APIAllowedCIDRBlocks is an allow list of CIDR blocks that can access the APIServer
+If not specified, traffic is allowed from all addresses.
+This depends on underlying support by the cloud provider for Service LoadBalancerSourceRanges</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>controllerAvailabilityPolicy</code></br>
 <em>
 <a href="#hypershift.openshift.io/v1alpha1.AvailabilityPolicy">

diff --git a/hack/app-sre/saas_template.yaml b/hack/app-sre/saas_template.yaml
@@ -20008,6 +20008,15 @@ objects:
                             associated with the loopback adapter of each node. If
                             not specified, 172.20.0.1 is used.
                           type: string
+                        allowedCIDRBlocks:
+                          description: AllowedCIDRBlocks is an allow list of CIDR
+                            blocks that can access the APIServer If not specified,
+                            traffic is allowed from all addresses. This depends on
+                            underlying support by the cloud provider for Service LoadBalancerSourceRanges
+                          items:
+                            pattern: ^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])(\/(3[0-2]|[1-2][0-9]|[0-9]))$
+                            type: string
+                          type: array
                         port:
                           description: Port is the port at which the APIServer is
                             exposed inside a node. Other pods using host networking
@@ -20930,6 +20939,15 @@ objects:
                   description: APIAdvertiseAddress is the address at which the APIServer
                     listens inside a worker.
                   type: string
+                apiAllowedCIDRBlocks:
+                  description: APIAllowedCIDRBlocks is an allow list of CIDR blocks
+                    that can access the APIServer If not specified, traffic is allowed
+                    from all addresses. This depends on underlying support by the
+                    cloud provider for Service LoadBalancerSourceRanges
+                  items:
+                    pattern: ^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])(\/(3[0-2]|[1-2][0-9]|[0-9]))$
+                    type: string
+                  type: array
                 apiPort:
                   description: APIPort is the port at which the APIServer listens
                     inside a worker

diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
@@ -1203,6 +1203,7 @@ func reconcileHostedControlPlane(hcp *hyperv1.HostedControlPlane, hcluster *hype
 	if hcluster.Spec.Networking.APIServer != nil {
 		hcp.Spec.APIAdvertiseAddress = hcluster.Spec.Networking.APIServer.AdvertiseAddress
 		hcp.Spec.APIPort = hcluster.Spec.Networking.APIServer.Port
+		hcp.Spec.APIAllowedCIDRBlocks = hcluster.Spec.Networking.APIServer.AllowedCIDRBlocks
 	}
 
 	hcp.Spec.ClusterID = hcluster.Spec.ClusterID