Merge pull request #165 from timuthy/feature.snat

Add useSNAT option to CloudProfile
gardener · Oct 22, 2020 · 88147e4 · 88147e4
2 parents 19f5108 + 25df2cf
commit 88147e4
Show file tree

Hide file tree

Showing 11 changed files with 47 additions and 0 deletions.
diff --git a/charts/internal/openstack-infra/templates/main.tf b/charts/internal/openstack-infra/templates/main.tf
@@ -27,6 +27,9 @@ resource "openstack_networking_router_v2" "router" {
   name                = "{{ required "clusterName is required" .Values.clusterName }}"
   region              = "{{ required "openstack.region is required" .Values.openstack.region }}"
   external_network_id = data.openstack_networking_network_v2.fip.id
+  {{ if .Values.router.enableSNAT -}}
+  enable_snat         = true
+  {{- end }}
   {{ if .Values.router.floatingPoolSubnetName -}}
   external_fixed_ip {
     subnet_id = data.openstack_networking_subnet_v2.fip_subnet.id

diff --git a/charts/internal/openstack-infra/values.yaml b/charts/internal/openstack-infra/values.yaml
@@ -12,6 +12,7 @@ sshPublicKey: sshkey-12345
 
 router:
   id: openstack_networking_router_v2.router.id
+  # enableSNAT: true
   # floatingPoolSubnetName: my-fip-subnet-name
 
 dnsServers:

diff --git a/docs/usage-as-operator.md b/docs/usage-as-operator.md
@@ -49,6 +49,7 @@ machineImages:
 # - 10.10.10.12
 # requestTimeout: 60s
 # useOctavia: true
+# useSNAT: true
 # rescanBlockStorageOnResize: true
 # nodeVolumeAttachLimit: 30
 constraints:
@@ -103,6 +104,8 @@ Some OpenStack environments don't need these regional mappings, hence, the `regi
 If your OpenStack environment only has regional values and it doesn't make sense to provide a (non-regional) fallback then simply
 omit `keystoneURL` and always specify `region`.
 
+If Gardener creates and manages the router of a shoot cluster, it is additionally possible to specify that the [enable_snat](https://registry.terraform.io/providers/terraform-provider-openstack/openstack/latest/docs/resources/networking_router_v2#enable_snat) field is set to `true` via `useSNAT: true` in the `CloudProfileConfig`.
+
 ## Example `CloudProfile` manifest
 
 Please find below an example `CloudProfile` manifest:

diff --git a/hack/api-reference/api.md b/hack/api-reference/api.md
@@ -176,6 +176,18 @@ bool
 <p>UseOctavia specifies whether the OpenStack Octavia network load balancing is used.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>useSNAT</code></br>
+<em>
+bool
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>UseSNAT specifies whether S-NAT is supposed to be used for the Gardener managed OpenStack router.</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="openstack.provider.extensions.gardener.cloud/v1alpha1.ControlPlaneConfig">ControlPlaneConfig

diff --git a/pkg/apis/openstack/types_cloudprofile.go b/pkg/apis/openstack/types_cloudprofile.go
@@ -49,6 +49,8 @@ type CloudProfileConfig struct {
 	NodeVolumeAttachLimit *int32
 	// UseOctavia specifies whether the OpenStack Octavia network load balancing is used.
 	UseOctavia *bool
+	// UseSNAT specifies whether S-NAT is supposed to be used for the Gardener managed OpenStack router.
+	UseSNAT *bool
 }
 
 // Constraints is an object containing constraints for the shoots.

diff --git a/pkg/apis/openstack/v1alpha1/types_cloudprofile.go b/pkg/apis/openstack/v1alpha1/types_cloudprofile.go
@@ -56,6 +56,9 @@ type CloudProfileConfig struct {
 	// UseOctavia specifies whether the OpenStack Octavia network load balancing is used.
 	// +optional
 	UseOctavia *bool `json:"useOctavia,omitempty"`
+	// UseSNAT specifies whether S-NAT is supposed to be used for the Gardener managed OpenStack router.
+	// +optional
+	UseSNAT *bool `json:"useSNAT,omitempty"`
 }
 
 // Constraints is an object containing constraints for the shoots.

diff --git a/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go b/pkg/apis/openstack/v1alpha1/zz_generated.conversion.go
diff --git a/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/openstack/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/apis/openstack/zz_generated.deepcopy.go b/pkg/apis/openstack/zz_generated.deepcopy.go
diff --git a/pkg/internal/infrastructure/terraform.go b/pkg/internal/infrastructure/terraform.go
@@ -102,6 +102,10 @@ func ComputeTerraformerChartValues(
 		return nil, err
 	}
 
+	if cloudProfileConfig.UseSNAT != nil {
+		routerConfig["enableSNAT"] = *cloudProfileConfig.UseSNAT
+	}
+
 	workersCIDR := config.Networks.Workers
 	// Backwards compatibility - remove this code in a future version.
 	if workersCIDR == "" {

diff --git a/pkg/internal/infrastructure/terraform_test.go b/pkg/internal/infrastructure/terraform_test.go
@@ -18,6 +18,8 @@ import (
 	"encoding/json"
 	"strconv"
 
+	"k8s.io/utils/pointer"
+
 	api "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack"
 	apiv1alpha1 "github.com/gardener/gardener-extension-provider-openstack/pkg/apis/openstack/v1alpha1"
 	"github.com/gardener/gardener-extension-provider-openstack/pkg/openstack"
@@ -162,9 +164,14 @@ var _ = Describe("Terraform", func() {
 		})
 
 		It("should correctly compute the terraformer chart values with vpc creation", func() {
+			cloudProfileConfig.UseSNAT = pointer.BoolPtr(true)
+			cloudProfileConfigJSON, _ = json.Marshal(cloudProfileConfig)
+			cluster.CloudProfile.Spec.ProviderConfig.Raw = cloudProfileConfigJSON
+
 			config.Networks.Router = nil
 			expectedCreateValues["router"] = true
 			expectedRouterValues["id"] = DefaultRouterID
+			expectedRouterValues["enableSNAT"] = true
 
 			values, err := ComputeTerraformerChartValues(infra, credentials, config, cluster)
 			Expect(err).To(BeNil())