diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml new file mode 100644 index 0000000000..5ded0c65ed --- /dev/null +++ b/.github/workflows/release.yaml @@ -0,0 +1,44 @@ +name: release + +on: + push: + # Sequence of patterns matched against refs/tags + tags: + - 'v*' # Push events to matching v*, i.e. v1.0, v20.15.10 + +permissions: + contents: write # Allow to create a release. + +jobs: + build: + name: create draft release + runs-on: ubuntu-latest + steps: + - name: Set env + run: echo "RELEASE_TAG=${GITHUB_REF:10}" >> $GITHUB_ENV + - name: checkout code + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2 + with: + fetch-depth: 0 + - name: Calculate go version + run: echo "go_version=$(make go-version)" >> $GITHUB_ENV + - name: Set up Go + uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # tag=v5.1.0 + with: + go-version: ${{ env.go_version }} + - name: generate release artifacts + run: | + make release + - name: generate release notes + # Ignore failures for release-notes generation so they could still get + # generated manually before publishing. + run: | + make generate-release-notes || echo "Failed to generate release notes" >> _releasenotes/${{ env.RELEASE_TAG }}.md + env: + GH_TOKEN: ${{ github.token }} + - name: Release + uses: softprops/action-gh-release@01570a1f39cb168c169c802c3bceb9e93fb10974 # tag=v2.1.0 + with: + draft: true + files: out/* + body_path: _releasenotes/${{ env.RELEASE_TAG }}.md diff --git a/.gitignore b/.gitignore index d2a0fe7cbf..f6a6f7bc78 100644 --- a/.gitignore +++ b/.gitignore @@ -74,6 +74,9 @@ cscope.* *.test /hack/.test-cmd-auth +# Generated release notes +_releasenotes + # JUnit test output from ginkgo e2e tests /junit*.xml diff --git a/Makefile b/Makefile index 07dc8e6c89..1e2caf0f15 100644 --- a/Makefile +++ b/Makefile @@ -19,10 +19,15 @@ include $(ROOT_DIR_RELATIVE)/common.mk # If you update this file, please follow # https://www.thapaliya.com/en/writings/well-documented-makefiles/ +export GOTOOLCHAIN=go1.22.8 + # Active module mode, as we use go modules to manage dependencies export GO111MODULE=on unexport GOPATH +# Go +GO_VERSION ?= 1.22.7 + # Directories. ARTIFACTS ?= $(REPO_ROOT)/_artifacts TOOLS_DIR := hack/tools @@ -357,8 +362,25 @@ staging-manifests: ##@ Release ## -------------------------------------- +ifneq (,$(findstring -,$(RELEASE_TAG))) + PRE_RELEASE=true +endif +PREVIOUS_TAG ?= $(shell git tag -l | grep -E "^v[0-9]+\.[0-9]+\.[0-9]+$$" | sort -V | grep -B1 $(RELEASE_TAG) | head -n 1 2>/dev/null) +## set by Prow, ref name of the base branch, e.g., main +RELEASE_DIR := out +RELEASE_NOTES_DIR := _releasenotes + +.PHONY: $(RELEASE_DIR) $(RELEASE_DIR): - mkdir -p $@ + mkdir -p $(RELEASE_DIR)/ + +.PHONY: $(RELEASE_NOTES_DIR) +$(RELEASE_NOTES_DIR): + mkdir -p $(RELEASE_NOTES_DIR)/ + +.PHONY: $(BUILD_DIR) +$(BUILD_DIR): + @mkdir -p $(BUILD_DIR) .PHONY: list-staging-releases list-staging-releases: ## List staging images for image promotion @@ -433,9 +455,14 @@ upload-gh-artifacts: $(GH) ## Upload artifacts to Github release release-alias-tag: # Adds the tag to the last build tag. gcloud container images add-tag -q $(CONTROLLER_IMG):$(TAG) $(CONTROLLER_IMG):$(RELEASE_ALIAS_TAG) -.PHONY: release-notes -release-notes: $(RELEASE_NOTES) ## Generate release notes - $(RELEASE_NOTES) $(RELEASE_NOTES_ARGS) +.PHONY: generate-release-notes ## Generate release notes +generate-release-notes: $(RELEASE_NOTES_DIR) $(RELEASE_NOTES) + # Reset the file + echo -n > $(RELEASE_NOTES_DIR)/$(RELEASE_TAG).md + if [ -n "${PRE_RELEASE}" ]; then \ + echo -e ":rotating_light: This is a RELEASE CANDIDATE. Use it only for testing purposes. If you find any bugs, file an [issue](https://github.com/kubernetes-sigs/cluster-api-provider-openstack/issues/new/choose).\n" >> $(RELEASE_NOTES_DIR)/$(RELEASE_TAG).md; \ + fi + "$(RELEASE_NOTES)" --from=$(PREVIOUS_TAG) >> $(RELEASE_NOTES_DIR)/$(RELEASE_TAG).md .PHONY: templates templates: ## Generate cluster templates @@ -555,3 +582,12 @@ compile-e2e: ## Test e2e compilation .PHONY: FORCE FORCE: + +## -------------------------------------- +## Helpers +## -------------------------------------- + +##@ helpers: + +go-version: ## Print the go version we use to compile our binaries and images + @echo $(GO_VERSION) diff --git a/RELEASE.md b/RELEASE.md index a86fd79693..df00a8103b 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -70,10 +70,7 @@ The content of the release notes differs depending on the type of release, speci for review and the promotion of the image. 1. Run `make release` to build artifacts to be attached to the GitHub release. 1. Generate and finalize the release notes and save them for the next step. - - Run `make release-notes RELEASE_NOTES_ARGS="--from "`. - - Depending on the type of release, substitute `` with the following: - - Stable releases: tag of the last stable release - - Pre-releases*: tag of the latest pre-release (or last stable release if there isn't one) + - Run `make release-notes`. - Pay close attention to the `## :question: Sort these by hand` section, as it contains items that need to be manually sorted. 1. Create a draft release in GitHub based on the tag created above - Name the release `Release [VERSION]` where VERSION is the full version string. diff --git a/api/v1alpha6/openstackmachine_conversion.go b/api/v1alpha6/openstackmachine_conversion.go index 47a757bd7d..8a52c5146f 100644 --- a/api/v1alpha6/openstackmachine_conversion.go +++ b/api/v1alpha6/openstackmachine_conversion.go @@ -184,8 +184,6 @@ func restorev1alpha6MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMa func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infrav1.OpenStackMachineSpec) { // PropagateUplinkStatus has been added in v1beta1. - // We restore the whole Ports since they are anyway immutable. - dst.Ports = previous.Ports dst.AdditionalBlockDevices = previous.AdditionalBlockDevices dst.ServerGroup = previous.ServerGroup dst.Image = previous.Image @@ -197,6 +195,12 @@ func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infr } } + if len(dst.Ports) == len(previous.Ports) { + for i := range dst.Ports { + restorev1beta1Port(&previous.Ports[i], &dst.Ports[i]) + } + } + if dst.RootVolume != nil && previous.RootVolume != nil { restorev1beta1BlockDeviceVolume( &previous.RootVolume.BlockDeviceVolume, diff --git a/api/v1alpha6/types_conversion.go b/api/v1alpha6/types_conversion.go index 30c8618cc8..def726cff1 100644 --- a/api/v1alpha6/types_conversion.go +++ b/api/v1alpha6/types_conversion.go @@ -330,11 +330,94 @@ func restorev1alpha6Port(previous *PortOpts, dst *PortOpts) { } } +func restorev1beta1Port(previous *infrav1.PortOpts, dst *infrav1.PortOpts) { + // PropagateUplinkStatus was not present in v1alpha6 + dst.PropagateUplinkStatus = previous.PropagateUplinkStatus + + optional.RestoreString(&previous.NameSuffix, &dst.NameSuffix) + optional.RestoreString(&previous.Description, &dst.Description) + optional.RestoreString(&previous.MACAddress, &dst.MACAddress) + + if len(dst.FixedIPs) == len(previous.FixedIPs) { + for j := range dst.FixedIPs { + prevFixedIP := &previous.FixedIPs[j] + dstFixedIP := &dst.FixedIPs[j] + + optional.RestoreString(&prevFixedIP.IPAddress, &dstFixedIP.IPAddress) + restorev1beta1SubnetParam(prevFixedIP.Subnet, dstFixedIP.Subnet) + } + } + + if len(dst.AllowedAddressPairs) == len(previous.AllowedAddressPairs) { + for j := range dst.AllowedAddressPairs { + prevAAP := &previous.AllowedAddressPairs[j] + dstAAP := &dst.AllowedAddressPairs[j] + + optional.RestoreString(&prevAAP.MACAddress, &dstAAP.MACAddress) + } + } + + optional.RestoreString(&previous.HostID, &dst.HostID) + optional.RestoreString(&previous.VNICType, &dst.VNICType) + + if previous.Profile != nil { + // A binding profile of {&false, &false} will be converted to a nil map. + // We still need to restore it, so substitute an empty BindProfile. + var dstProfile *infrav1.BindingProfile + if dst.Profile != nil { + dstProfile = dst.Profile + } else { + dstProfile = &infrav1.BindingProfile{} + dst.Profile = dstProfile + } + prevProfile := previous.Profile + + if dstProfile.OVSHWOffload == nil || !*dstProfile.OVSHWOffload { + dstProfile.OVSHWOffload = prevProfile.OVSHWOffload + } + + if dstProfile.TrustedVF == nil || !*dstProfile.TrustedVF { + dstProfile.TrustedVF = prevProfile.TrustedVF + } + } +} + func Convert_v1alpha6_PortOpts_To_v1beta1_PortOpts(in *PortOpts, out *infrav1.PortOpts, s apiconversion.Scope) error { if err := autoConvert_v1alpha6_PortOpts_To_v1beta1_PortOpts(in, out, s); err != nil { return err } + // Copy members of ResolvedPortSpecFields + var allowedAddressPairs []infrav1.AddressPair + if len(in.AllowedAddressPairs) > 0 { + allowedAddressPairs = make([]infrav1.AddressPair, len(in.AllowedAddressPairs)) + for i := range in.AllowedAddressPairs { + aap := &in.AllowedAddressPairs[i] + allowedAddressPairs[i] = infrav1.AddressPair{ + MACAddress: &aap.MACAddress, + IPAddress: aap.IPAddress, + } + } + } + var valueSpecs []infrav1.ValueSpec + if len(in.ValueSpecs) > 0 { + valueSpecs = make([]infrav1.ValueSpec, len(in.ValueSpecs)) + for i, vs := range in.ValueSpecs { + valueSpecs[i] = infrav1.ValueSpec(vs) + } + } + out.AdminStateUp = in.AdminStateUp + out.AllowedAddressPairs = allowedAddressPairs + out.DisablePortSecurity = in.DisablePortSecurity + out.ValueSpecs = valueSpecs + if err := errors.Join( + optional.Convert_string_To_optional_String(&in.MACAddress, &out.MACAddress, s), + optional.Convert_string_To_optional_String(&in.HostID, &out.HostID, s), + optional.Convert_string_To_optional_String(&in.VNICType, &out.VNICType, s), + ); err != nil { + return err + } + if len(in.SecurityGroups) > 0 || len(in.SecurityGroupFilters) > 0 { out.SecurityGroups = make([]infrav1.SecurityGroupParam, len(in.SecurityGroups)+len(in.SecurityGroupFilters)) for i := range in.SecurityGroupFilters { @@ -373,6 +456,38 @@ func Convert_v1beta1_PortOpts_To_v1alpha6_PortOpts(in *infrav1.PortOpts, out *Po return err } + // Copy members of ResolvedPortSpecFields + var allowedAddressPairs []AddressPair + if len(in.AllowedAddressPairs) > 0 { + allowedAddressPairs = make([]AddressPair, len(in.AllowedAddressPairs)) + for i := range in.AllowedAddressPairs { + inAAP := &in.AllowedAddressPairs[i] + outAAP := &allowedAddressPairs[i] + if err := optional.Convert_optional_String_To_string(&inAAP.MACAddress, &outAAP.MACAddress, s); err != nil { + return err + } + outAAP.IPAddress = inAAP.IPAddress + } + } + var valueSpecs []ValueSpec + if len(in.ValueSpecs) > 0 { + valueSpecs = make([]ValueSpec, len(in.ValueSpecs)) + for i, vs := range in.ValueSpecs { + valueSpecs[i] = ValueSpec(vs) + } + } + out.AdminStateUp = in.AdminStateUp + out.AllowedAddressPairs = allowedAddressPairs + out.DisablePortSecurity = in.DisablePortSecurity + out.ValueSpecs = valueSpecs + if err := errors.Join( + optional.Convert_optional_String_To_string(&in.MACAddress, &out.MACAddress, s), + optional.Convert_optional_String_To_string(&in.HostID, &out.HostID, s), + optional.Convert_optional_String_To_string(&in.VNICType, &out.VNICType, s), + ); err != nil { + return err + } + // The auto-generated function converts v1beta1 SecurityGroup to // v1alpha6 SecurityGroup, but v1alpha6 SecurityGroupFilter is more // appropriate. Unset them and convert to SecurityGroupFilter instead. diff --git a/api/v1beta1/openstackcluster_types.go b/api/v1beta1/openstackcluster_types.go index d6745e5ccd..5da4d77111 100644 --- a/api/v1beta1/openstackcluster_types.go +++ b/api/v1beta1/openstackcluster_types.go @@ -31,6 +31,8 @@ const ( ) // OpenStackClusterSpec defines the desired state of OpenStackCluster. +// +kubebuilder:validation:XValidation:rule="has(self.disableExternalNetwork) && self.disableExternalNetwork ? !has(self.bastion) || !has(self.bastion.floatingIP) : true",message="bastion floating IP cannot be set when disableExternalNetwork is true" +// +kubebuilder:validation:XValidation:rule="has(self.disableExternalNetwork) && self.disableExternalNetwork ? has(self.disableAPIServerFloatingIP) && self.disableAPIServerFloatingIP : true",message="disableAPIServerFloatingIP cannot be false when disableExternalNetwork is true" type OpenStackClusterSpec struct { // ManagedSubnets describe OpenStack Subnets to be created. Cluster actuator will create a network, // subnets with the defined CIDR, and a router connected to these subnets. Currently only one IPv4 diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index fd240c51f5..80778a76da 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -6570,6 +6570,16 @@ spec: required: - identityRef type: object + x-kubernetes-validations: + - message: bastion floating IP cannot be set when disableExternalNetwork + is true + rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork + ? !has(self.bastion) || !has(self.bastion.floatingIP) : true' + - message: disableAPIServerFloatingIP cannot be false when disableExternalNetwork + is true + rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork + ? has(self.disableAPIServerFloatingIP) && self.disableAPIServerFloatingIP + : true' status: description: OpenStackClusterStatus defines the observed state of OpenStackCluster. properties: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index 96680265b6..3155b8c054 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -4027,6 +4027,16 @@ spec: required: - identityRef type: object + x-kubernetes-validations: + - message: bastion floating IP cannot be set when disableExternalNetwork + is true + rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork + ? !has(self.bastion) || !has(self.bastion.floatingIP) : true' + - message: disableAPIServerFloatingIP cannot be false when disableExternalNetwork + is true + rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork + ? has(self.disableAPIServerFloatingIP) && self.disableAPIServerFloatingIP + : true' required: - spec type: object diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index d310aced6a..96846f58ca 100644 --- a/controllers/openstackcluster_controller.go +++ b/controllers/openstackcluster_controller.go @@ -507,7 +507,11 @@ func reconcileBastion(scope *scope.WithLogger, cluster *clusterv1.Cluster, openS return nil, err } - return bastionAddFloatingIP(openStackCluster, clusterResourceName, port, networkingService) + if !ptr.Deref(openStackCluster.Spec.DisableExternalNetwork, false) { + return bastionAddFloatingIP(openStackCluster, clusterResourceName, port, networkingService) + } + + return nil, nil } func bastionAddFloatingIP(openStackCluster *infrav1.OpenStackCluster, clusterResourceName string, port *ports.Port, networkingService *networking.Service) (*reconcile.Result, error) { @@ -564,6 +568,12 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, cluster * } machineSpec := bastion.Spec + + // Create metadata map from ServerMetadata + metadata := make(map[string]string) + for _, item := range bastion.Spec.ServerMetadata { + metadata[item.Key] = item.Value + } instanceSpec := &compute.InstanceSpec{ Name: bastionName(cluster.Name), Flavor: machineSpec.Flavor, @@ -572,6 +582,7 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, cluster * RootVolume: machineSpec.RootVolume, ServerGroupID: resolved.ServerGroupID, Tags: compute.InstanceTags(machineSpec, openStackCluster), + Metadata: metadata, } if bastion.AvailabilityZone != nil { instanceSpec.FailureDomain = *bastion.AvailabilityZone @@ -834,9 +845,9 @@ func reconcileControlPlaneEndpoint(scope *scope.WithLogger, networkingService *n case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): host = openStackCluster.Spec.ControlPlaneEndpoint.Host - // API server load balancer is disabled, but floating IP is not. Create + // API server load balancer is disabled, but external netowork and floating IP are not. Create // a floating IP to be attached directly to a control plane host. - case !ptr.Deref(openStackCluster.Spec.DisableAPIServerFloatingIP, false): + case !ptr.Deref(openStackCluster.Spec.DisableAPIServerFloatingIP, false) && !ptr.Deref(openStackCluster.Spec.DisableExternalNetwork, false): fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterResourceName, openStackCluster.Spec.APIServerFloatingIP) if err != nil { handleUpdateOSCError(openStackCluster, fmt.Errorf("floating IP cannot be got or created: %w", err)) diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index 230ecd5342..8638b72f5a 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -708,7 +708,7 @@ func (r *OpenStackMachineReconciler) reconcileAPIServerLoadBalancer(scope *scope conditions.MarkFalse(openStackMachine, infrav1.APIServerIngressReadyCondition, infrav1.LoadBalancerMemberErrorReason, clusterv1.ConditionSeverityError, "Reconciling load balancer member failed: %v", err) return fmt.Errorf("reconcile load balancer member: %w", err) } - } else if !ptr.Deref(openStackCluster.Spec.DisableAPIServerFloatingIP, false) { + } else if !ptr.Deref(openStackCluster.Spec.DisableAPIServerFloatingIP, false) && !ptr.Deref(openStackCluster.Spec.DisableExternalNetwork, false) { var floatingIPAddress *string switch { case openStackCluster.Spec.ControlPlaneEndpoint != nil && openStackCluster.Spec.ControlPlaneEndpoint.IsValid(): diff --git a/docs/book/src/clusteropenstack/configuration.md b/docs/book/src/clusteropenstack/configuration.md index 193720927e..3a0439c584 100644 --- a/docs/book/src/clusteropenstack/configuration.md +++ b/docs/book/src/clusteropenstack/configuration.md @@ -270,6 +270,8 @@ associated to the first controller node and the other controller nodes have no f to any other controller node. So we recommend to only set one controller node when floating IP is needed, or please consider using load balancer instead, see [issue #1265](https://github.com/kubernetes-sigs/cluster-api-provider-openstack/issues/1265) for further information. +Note: `spec.disableExternalNetwork` must be unset or set to `false` to allow the API server to have a floating IP. + ### Disabling the API server floating IP It is possible to provision a cluster without a floating IP for the API server by setting @@ -715,6 +717,8 @@ spec: floatingIP: ``` +Note: A floating IP can only be added if `OpenStackCluster.Spec.DisableExternalNetwork` is not set or set to `false`. + If `managedSecurityGroups` is set to a non-nil value (e.g. `{}`), security group rule opening 22/tcp is added to security groups for bastion, controller, and worker nodes respectively. Otherwise, you have to add `securityGroups` to the `bastion` in `OpenStackCluster` spec and `OpenStackMachineTemplate` spec template respectively. ### Making changes to the bastion host diff --git a/openshift/manifests/0000_30_cluster-api-provider-openstack_04_infrastructure-components.yaml b/openshift/manifests/0000_30_cluster-api-provider-openstack_04_infrastructure-components.yaml index 8bf587f807..1a7f888d98 100644 --- a/openshift/manifests/0000_30_cluster-api-provider-openstack_04_infrastructure-components.yaml +++ b/openshift/manifests/0000_30_cluster-api-provider-openstack_04_infrastructure-components.yaml @@ -6590,6 +6590,16 @@ data: required: - identityRef type: object + x-kubernetes-validations: + - message: bastion floating IP cannot be set when disableExternalNetwork + is true + rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork + ? !has(self.bastion) || !has(self.bastion.floatingIP) : true' + - message: disableAPIServerFloatingIP cannot be false when disableExternalNetwork + is true + rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork + ? has(self.disableAPIServerFloatingIP) && self.disableAPIServerFloatingIP + : true' status: description: OpenStackClusterStatus defines the observed state of OpenStackCluster. properties: @@ -11124,6 +11134,16 @@ data: required: - identityRef type: object + x-kubernetes-validations: + - message: bastion floating IP cannot be set when disableExternalNetwork + is true + rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork + ? !has(self.bastion) || !has(self.bastion.floatingIP) : true' + - message: disableAPIServerFloatingIP cannot be false when disableExternalNetwork + is true + rule: 'has(self.disableExternalNetwork) && self.disableExternalNetwork + ? has(self.disableAPIServerFloatingIP) && self.disableAPIServerFloatingIP + : true' required: - spec type: object diff --git a/openshift/vendor/sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1/openstackcluster_types.go b/openshift/vendor/sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1/openstackcluster_types.go index d6745e5ccd..5da4d77111 100644 --- a/openshift/vendor/sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1/openstackcluster_types.go +++ b/openshift/vendor/sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1/openstackcluster_types.go @@ -31,6 +31,8 @@ const ( ) // OpenStackClusterSpec defines the desired state of OpenStackCluster. +// +kubebuilder:validation:XValidation:rule="has(self.disableExternalNetwork) && self.disableExternalNetwork ? !has(self.bastion) || !has(self.bastion.floatingIP) : true",message="bastion floating IP cannot be set when disableExternalNetwork is true" +// +kubebuilder:validation:XValidation:rule="has(self.disableExternalNetwork) && self.disableExternalNetwork ? has(self.disableAPIServerFloatingIP) && self.disableAPIServerFloatingIP : true",message="disableAPIServerFloatingIP cannot be false when disableExternalNetwork is true" type OpenStackClusterSpec struct { // ManagedSubnets describe OpenStack Subnets to be created. Cluster actuator will create a network, // subnets with the defined CIDR, and a router connected to these subnets. Currently only one IPv4 diff --git a/pkg/cloud/services/networking/floatingip.go b/pkg/cloud/services/networking/floatingip.go index 4e611f7fe0..dc474b846d 100644 --- a/pkg/cloud/services/networking/floatingip.go +++ b/pkg/cloud/services/networking/floatingip.go @@ -38,6 +38,11 @@ func (s *Service) GetOrCreateFloatingIP(eventObject runtime.Object, openStackClu var err error var fpCreateOpts floatingips.CreateOpts + // This is a safeguard, we shouldn't reach it and if we do, it's something to fix in the caller of the function. + if openStackCluster.Status.ExternalNetwork == nil { + return nil, fmt.Errorf("external network not found") + } + if ptr.Deref(ip, "") != "" { fp, err = s.GetFloatingIP(*ip) if err != nil { diff --git a/test/e2e/suites/apivalidations/openstackcluster_test.go b/test/e2e/suites/apivalidations/openstackcluster_test.go index 76bcbb40bf..943af0605c 100644 --- a/test/e2e/suites/apivalidations/openstackcluster_test.go +++ b/test/e2e/suites/apivalidations/openstackcluster_test.go @@ -169,6 +169,29 @@ var _ = Describe("OpenStackCluster API validations", func() { } Expect(createObj(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed") }) + + It("should not allow a bastion floating IP with DisableExternalNetwork set to true", func() { + cluster.Spec.Bastion = &infrav1.Bastion{ + Enabled: ptr.To(true), + Spec: &infrav1.OpenStackMachineSpec{ + Flavor: "flavor-name", + Image: infrav1.ImageParam{ + Filter: &infrav1.ImageFilter{ + Name: ptr.To("fake-image"), + }, + }, + }, + FloatingIP: ptr.To("10.0.0.0"), + } + cluster.Spec.DisableExternalNetwork = ptr.To(true) + Expect(createObj(cluster)).ToNot(Succeed(), "OpenStackCluster creation should not succeed") + }) + + It("should not allow DisableAPIServerFloatingIP to be false with DisableExternalNetwork set to true", func() { + cluster.Spec.DisableAPIServerFloatingIP = ptr.To(false) + cluster.Spec.DisableExternalNetwork = ptr.To(true) + Expect(createObj(cluster)).ToNot(Succeed(), "OpenStackCluster creation should not succeed") + }) }) Context("v1alpha7", func() {