Skip to content

Commit

Permalink
add archv1 check to multiple ip static validation (#3406)
Browse files Browse the repository at this point in the history 
* add archv1 check to multiple ip static validation

* add archv1 check to preview api static validation

* use InstallArchitectureVersion constant in test
  • Loading branch information
@tony-schndr
tony-schndr authored Feb 19, 2024
1 parent 7f99749 commit f4d03fb
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 16 deletions.
24 changes: 16 additions & 8 deletions pkg/api/v20230701preview/openshiftcluster_validatestatic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/Azure/ARO-RP/pkg/api/validate"
"github.com/Azure/ARO-RP/pkg/util/pullsecret"
"github.com/Azure/ARO-RP/pkg/util/uuid"
"github.com/Azure/ARO-RP/pkg/util/version"
)

type openShiftClusterStaticValidator struct {
Expand All @@ -35,12 +36,16 @@ func (sv openShiftClusterStaticValidator) Static(_oc interface{}, _current *api.
sv.domain = domain
sv.requireD2sV3Workers = requireD2sV3Workers
sv.resourceID = resourceID
var architectureVersion api.ArchitectureVersion

oc := _oc.(*OpenShiftCluster)

var current *OpenShiftCluster
if _current != nil {
architectureVersion = _current.Properties.ArchitectureVersion
current = (&openShiftClusterConverter{}).ToExternal(_current).(*OpenShiftCluster)
} else {
architectureVersion = version.InstallArchitectureVersion
}

var err error
Expand All @@ -49,7 +54,7 @@ func (sv openShiftClusterStaticValidator) Static(_oc interface{}, _current *api.
return err
}

err = sv.validate(oc, current == nil)
err = sv.validate(oc, current == nil, architectureVersion)
if err != nil {
return err
}
Expand All @@ -61,7 +66,7 @@ func (sv openShiftClusterStaticValidator) Static(_oc interface{}, _current *api.
return sv.validateDelta(oc, current)
}

func (sv openShiftClusterStaticValidator) validate(oc *OpenShiftCluster, isCreate bool) error {
func (sv openShiftClusterStaticValidator) validate(oc *OpenShiftCluster, isCreate bool, architectureVersion api.ArchitectureVersion) error {
if !strings.EqualFold(oc.ID, sv.resourceID) {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeMismatchingResourceID, "id", "The provided resource ID '%s' did not match the name in the Url '%s'.", oc.ID, sv.resourceID)
}
Expand All @@ -75,10 +80,10 @@ func (sv openShiftClusterStaticValidator) validate(oc *OpenShiftCluster, isCreat
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "location", "The provided location '%s' is invalid.", oc.Location)
}

return sv.validateProperties("properties", &oc.Properties, isCreate)
return sv.validateProperties("properties", &oc.Properties, isCreate, architectureVersion)
}

func (sv openShiftClusterStaticValidator) validateProperties(path string, p *OpenShiftClusterProperties, isCreate bool) error {
func (sv openShiftClusterStaticValidator) validateProperties(path string, p *OpenShiftClusterProperties, isCreate bool, architectureVersion api.ArchitectureVersion) error {
switch p.ProvisioningState {
case ProvisioningStateCreating, ProvisioningStateUpdating,
ProvisioningStateAdminUpdating, ProvisioningStateDeleting,
Expand All @@ -98,7 +103,7 @@ func (sv openShiftClusterStaticValidator) validateProperties(path string, p *Ope
if err := sv.validateNetworkProfile(path+".networkProfile", &p.NetworkProfile, p.APIServerProfile.Visibility, p.IngressProfiles[0].Visibility); err != nil {
return err
}
if err := sv.validateLoadBalancerProfile(path+".networkProfile.loadBalancerProfile", p.NetworkProfile.LoadBalancerProfile, isCreate); err != nil {
if err := sv.validateLoadBalancerProfile(path+".networkProfile.loadBalancerProfile", p.NetworkProfile.LoadBalancerProfile, isCreate, architectureVersion); err != nil {
return err
}
if err := sv.validateMasterProfile(path+".masterProfile", &p.MasterProfile); err != nil {
Expand Down Expand Up @@ -253,7 +258,7 @@ func (sv openShiftClusterStaticValidator) validateNetworkProfile(path string, np
return nil
}

func (sv openShiftClusterStaticValidator) validateLoadBalancerProfile(path string, lbp *LoadBalancerProfile, isCreate bool) error {
func (sv openShiftClusterStaticValidator) validateLoadBalancerProfile(path string, lbp *LoadBalancerProfile, isCreate bool, architectureVersion api.ArchitectureVersion) error {
if lbp == nil {
return nil
}
Expand All @@ -265,7 +270,7 @@ func (sv openShiftClusterStaticValidator) validateLoadBalancerProfile(path strin

switch {
case lbp.ManagedOutboundIPs != nil:
err := validateManagedOutboundIPs(path, *lbp.ManagedOutboundIPs)
err := validateManagedOutboundIPs(path, *lbp.ManagedOutboundIPs, architectureVersion)
if err != nil {
return err
}
Expand Down Expand Up @@ -308,7 +313,10 @@ func checkPickedExactlyOne(path string, lbp *LoadBalancerProfile) error {
return nil
}

func validateManagedOutboundIPs(path string, managedOutboundIPs ManagedOutboundIPs) error {
func validateManagedOutboundIPs(path string, managedOutboundIPs ManagedOutboundIPs, architectureVersion api.ArchitectureVersion) error {
if architectureVersion == api.ArchitectureVersionV1 && managedOutboundIPs.Count > 1 {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".managedOutboundIps.count", "The provided managedOutboundIps.count %d is invalid: managedOutboundIps.count must be 1, multiple IPs are not supported for this cluster's network architecture.", managedOutboundIPs.Count)
}
if !(managedOutboundIPs.Count > 0 && managedOutboundIPs.Count <= 20) {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".managedOutboundIps.count", "The provided managedOutboundIps.count %d is invalid: managedOutboundIps.count must be in the range of 1 to 20 (inclusive).", managedOutboundIPs.Count)
}
Expand Down
25 changes: 25 additions & 0 deletions pkg/api/v20230701preview/openshiftcluster_validatestatic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/util/uuid"
"github.com/Azure/ARO-RP/pkg/util/version"
"github.com/Azure/ARO-RP/test/validate"
)

Expand All @@ -25,6 +26,7 @@ type validateTest struct {
current func(oc *OpenShiftCluster)
modify func(oc *OpenShiftCluster)
requireD2sV3Workers bool
architectureVersion *api.ArchitectureVersion
wantErr string
}

Expand Down Expand Up @@ -129,6 +131,10 @@ func runTests(t *testing.T, mode testMode, tests []*validateTest) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// default values if not set
if tt.architectureVersion == nil {
tt.architectureVersion = (*api.ArchitectureVersion)(to.IntPtr(int(version.InstallArchitectureVersion)))
}

if tt.location == nil {
tt.location = to.StringPtr("location")
}
Expand Down Expand Up @@ -167,6 +173,8 @@ func runTests(t *testing.T, mode testMode, tests []*validateTest) {
var current *api.OpenShiftCluster
if mode == testModeUpdate {
current = &api.OpenShiftCluster{}
current.Properties.ArchitectureVersion = *tt.architectureVersion

(&openShiftClusterConverter{}).ToInternal(validOCForTest(), current)
}

Expand Down Expand Up @@ -744,9 +752,26 @@ func TestOpenShiftClusterStaticValidateLoadBalancerProfile(t *testing.T) {
wantErr: "400: InvalidParameter: properties.networkProfile.loadBalancerProfile.effectiveOutboundIps: The field effectiveOutboundIps is read only.",
},
}

updateOnlyTests := []*validateTest{
{
name: "LoadBalancerProfile.ManagedOutboundIPs is invalid with multiple managed IPs and architecture v1",
current: func(oc *OpenShiftCluster) {
oc.Properties.NetworkProfile.LoadBalancerProfile = &LoadBalancerProfile{
ManagedOutboundIPs: &ManagedOutboundIPs{
Count: 20,
},
}
},
architectureVersion: (*api.ArchitectureVersion)(to.IntPtr(int(api.ArchitectureVersionV1))),
wantErr: "400: InvalidParameter: properties.networkProfile.loadBalancerProfile.managedOutboundIps.count: The provided managedOutboundIps.count 20 is invalid: managedOutboundIps.count must be 1, multiple IPs are not supported for this cluster's network architecture.",
},
}

runTests(t, testModeCreate, createTests)
runTests(t, testModeCreate, tests)
runTests(t, testModeUpdate, tests)
runTests(t, testModeUpdate, updateOnlyTests)
}

func TestOpenShiftClusterStaticValidateMasterProfile(t *testing.T) {
Expand Down
24 changes: 16 additions & 8 deletions pkg/api/v20231122/openshiftcluster_validatestatic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/Azure/ARO-RP/pkg/api/validate"
"github.com/Azure/ARO-RP/pkg/util/pullsecret"
"github.com/Azure/ARO-RP/pkg/util/uuid"
"github.com/Azure/ARO-RP/pkg/util/version"
)

type openShiftClusterStaticValidator struct {
Expand All @@ -35,12 +36,16 @@ func (sv openShiftClusterStaticValidator) Static(_oc interface{}, _current *api.
sv.domain = domain
sv.requireD2sV3Workers = requireD2sV3Workers
sv.resourceID = resourceID
var architectureVersion api.ArchitectureVersion

oc := _oc.(*OpenShiftCluster)

var current *OpenShiftCluster
if _current != nil {
architectureVersion = _current.Properties.ArchitectureVersion
current = (&openShiftClusterConverter{}).ToExternal(_current).(*OpenShiftCluster)
} else {
architectureVersion = version.InstallArchitectureVersion
}

var err error
Expand All @@ -49,7 +54,7 @@ func (sv openShiftClusterStaticValidator) Static(_oc interface{}, _current *api.
return err
}

err = sv.validate(oc, current == nil)
err = sv.validate(oc, current == nil, architectureVersion)
if err != nil {
return err
}
Expand All @@ -61,7 +66,7 @@ func (sv openShiftClusterStaticValidator) Static(_oc interface{}, _current *api.
return sv.validateDelta(oc, current)
}

func (sv openShiftClusterStaticValidator) validate(oc *OpenShiftCluster, isCreate bool) error {
func (sv openShiftClusterStaticValidator) validate(oc *OpenShiftCluster, isCreate bool, architectureVersion api.ArchitectureVersion) error {
if !strings.EqualFold(oc.ID, sv.resourceID) {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeMismatchingResourceID, "id", "The provided resource ID '%s' did not match the name in the Url '%s'.", oc.ID, sv.resourceID)
}
Expand All @@ -75,10 +80,10 @@ func (sv openShiftClusterStaticValidator) validate(oc *OpenShiftCluster, isCreat
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "location", "The provided location '%s' is invalid.", oc.Location)
}

return sv.validateProperties("properties", &oc.Properties, isCreate)
return sv.validateProperties("properties", &oc.Properties, isCreate, architectureVersion)
}

func (sv openShiftClusterStaticValidator) validateProperties(path string, p *OpenShiftClusterProperties, isCreate bool) error {
func (sv openShiftClusterStaticValidator) validateProperties(path string, p *OpenShiftClusterProperties, isCreate bool, architectureVersion api.ArchitectureVersion) error {
switch p.ProvisioningState {
case ProvisioningStateCreating, ProvisioningStateUpdating,
ProvisioningStateAdminUpdating, ProvisioningStateDeleting,
Expand All @@ -98,7 +103,7 @@ func (sv openShiftClusterStaticValidator) validateProperties(path string, p *Ope
if err := sv.validateNetworkProfile(path+".networkProfile", &p.NetworkProfile, p.APIServerProfile.Visibility, p.IngressProfiles[0].Visibility); err != nil {
return err
}
if err := sv.validateLoadBalancerProfile(path+".networkProfile.loadBalancerProfile", p.NetworkProfile.LoadBalancerProfile, isCreate); err != nil {
if err := sv.validateLoadBalancerProfile(path+".networkProfile.loadBalancerProfile", p.NetworkProfile.LoadBalancerProfile, isCreate, architectureVersion); err != nil {
return err
}
if err := sv.validateMasterProfile(path+".masterProfile", &p.MasterProfile); err != nil {
Expand Down Expand Up @@ -243,14 +248,14 @@ func (sv openShiftClusterStaticValidator) validateNetworkProfile(path string, np
return nil
}

func (sv openShiftClusterStaticValidator) validateLoadBalancerProfile(path string, lbp *LoadBalancerProfile, isCreate bool) error {
func (sv openShiftClusterStaticValidator) validateLoadBalancerProfile(path string, lbp *LoadBalancerProfile, isCreate bool, architectureVersion api.ArchitectureVersion) error {
if lbp == nil {
return nil
}

switch {
case lbp.ManagedOutboundIPs != nil:
err := validateManagedOutboundIPs(path, *lbp.ManagedOutboundIPs)
err := validateManagedOutboundIPs(path, *lbp.ManagedOutboundIPs, architectureVersion)
if err != nil {
return err
}
Expand All @@ -263,7 +268,10 @@ func (sv openShiftClusterStaticValidator) validateLoadBalancerProfile(path strin
return nil
}

func validateManagedOutboundIPs(path string, managedOutboundIPs ManagedOutboundIPs) error {
func validateManagedOutboundIPs(path string, managedOutboundIPs ManagedOutboundIPs, architectureVersion api.ArchitectureVersion) error {
if architectureVersion == api.ArchitectureVersionV1 && managedOutboundIPs.Count > 1 {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".managedOutboundIps.count", "The provided managedOutboundIps.count %d is invalid: managedOutboundIps.count must be 1, multiple IPs are not supported for this cluster's network architecture.", managedOutboundIPs.Count)
}
if !(managedOutboundIPs.Count > 0 && managedOutboundIPs.Count <= 20) {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".managedOutboundIps.count", "The provided managedOutboundIps.count %d is invalid: managedOutboundIps.count must be in the range of 1 to 20 (inclusive).", managedOutboundIPs.Count)
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/api/v20231122/openshiftcluster_validatestatic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type validateTest struct {
current func(oc *OpenShiftCluster)
modify func(oc *OpenShiftCluster)
requireD2sV3Workers bool
architectureVersion *api.ArchitectureVersion
wantErr string
}

Expand Down Expand Up @@ -138,6 +139,10 @@ func runTests(t *testing.T, mode testMode, tests []*validateTest) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// default values if not set
if tt.architectureVersion == nil {
tt.architectureVersion = (*api.ArchitectureVersion)(to.IntPtr(int(version.InstallArchitectureVersion)))
}

if tt.location == nil {
tt.location = to.StringPtr("location")
}
Expand Down Expand Up @@ -183,6 +188,7 @@ func runTests(t *testing.T, mode testMode, tests []*validateTest) {
ext.Properties.APIServerProfile.URL = apiserverProfileUrl
ext.Properties.APIServerProfile.IP = apiserverProfileIp
ext.Properties.IngressProfiles[0].IP = ingressProfileIp
current.Properties.ArchitectureVersion = *tt.architectureVersion

(&openShiftClusterConverter{}).ToInternal(ext, current)
}
Expand Down Expand Up @@ -588,6 +594,7 @@ func TestOpenShiftClusterStaticValidateLoadBalancerProfile(t *testing.T) {
name: "LoadBalancerProfile is valid",
wantErr: "",
},

{
name: "LoadBalancerProfile.ManagedOutboundIPs is valid with 20 managed IPs",
current: func(oc *OpenShiftCluster) {
Expand Down Expand Up @@ -641,9 +648,26 @@ func TestOpenShiftClusterStaticValidateLoadBalancerProfile(t *testing.T) {
wantErr: "400: InvalidParameter: properties.networkProfile.loadBalancerProfile.effectiveOutboundIps: The field effectiveOutboundIps is read only.",
},
}

updateOnlyTests := []*validateTest{
{
name: "LoadBalancerProfile.ManagedOutboundIPs is invalid with multiple managed IPs and architecture v1",
current: func(oc *OpenShiftCluster) {
oc.Properties.NetworkProfile.LoadBalancerProfile = &LoadBalancerProfile{
ManagedOutboundIPs: &ManagedOutboundIPs{
Count: 20,
},
}
},
architectureVersion: (*api.ArchitectureVersion)(to.IntPtr(int(api.ArchitectureVersionV1))),
wantErr: "400: InvalidParameter: properties.networkProfile.loadBalancerProfile.managedOutboundIps.count: The provided managedOutboundIps.count 20 is invalid: managedOutboundIps.count must be 1, multiple IPs are not supported for this cluster's network architecture.",
},
}

runTests(t, testModeCreate, createTests)
runTests(t, testModeCreate, tests)
runTests(t, testModeUpdate, tests)
runTests(t, testModeUpdate, updateOnlyTests)
}

func TestOpenShiftClusterStaticValidateMasterProfile(t *testing.T) {
Expand Down

0 comments on commit f4d03fb

Please sign in to comment.