Skip to content

Commit 9b07817

Browse files
patrickdillonpatrickdillon
committed
Validate install config with feature gates
Enable checking install config fields with the feature gate package. Allows install config fields to be validated with CustomNoUpgrade feature sets in addition to the TechPreviewNoUpgrade feature set.
1 parent e6efecc commit 9b07817

File tree

5 files changed

+237
-26
lines changed

5 files changed

+237
-26
lines changed

‎pkg/types/gcp/validation/featuregates.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package validation
2+
3+
import (
4+
"k8s.io/apimachinery/pkg/util/validation/field"
5+
6+
configv1 "github.com/openshift/api/config/v1"
7+
"github.com/openshift/installer/pkg/types"
8+
"github.com/openshift/installer/pkg/types/featuregates"
9+
)
10+
11+
// GatedFeatures determines all of the install config fields that should
12+
// be validated to ensure that the proper featuregate is enabled when the field is used.
13+
func GatedFeatures(c *types.InstallConfig) []featuregates.GatedInstallConfigFeature {
14+
g := c.GCP
15+
return []featuregates.GatedInstallConfigFeature{
16+
{
17+
FeatureGateName: configv1.FeatureGateGCPLabelsTags,
18+
Condition: len(g.UserLabels) > 0,
19+
Field: field.NewPath("platform", "gcp", "userLabels"),
20+
},
21+
{
22+
FeatureGateName: configv1.FeatureGateGCPLabelsTags,
23+
Condition: len(g.UserTags) > 0,
24+
Field: field.NewPath("platform", "gcp", "userTags"),
25+
},
26+
}
27+
}

‎pkg/types/installconfig.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/openshift/installer/pkg/types/azure"
1414
"github.com/openshift/installer/pkg/types/baremetal"
1515
"github.com/openshift/installer/pkg/types/external"
16+
"github.com/openshift/installer/pkg/types/featuregates"
1617
"github.com/openshift/installer/pkg/types/gcp"
1718
"github.com/openshift/installer/pkg/types/ibmcloud"
1819
"github.com/openshift/installer/pkg/types/libvirt"
@@ -516,3 +517,22 @@ func (c *InstallConfig) WorkerMachinePool() *MachinePool {
516517

517518
return nil
518519
}
520+
521+
// EnabledFeatureGates returns a FeatureGate that can be checked (using the Enabled function)
522+
// to determine if a feature gate is enabled in the current feature sets.
523+
func (c *InstallConfig) EnabledFeatureGates() (featuregates.FeatureGate, error) {
524+
var customFS *configv1.CustomFeatureGates
525+
var err error
526+
if c.FeatureSet == configv1.CustomNoUpgrade {
527+
customFS, err = featuregates.GenerateCustomFeatures(c.FeatureGates)
528+
}
529+
if err != nil {
530+
return nil, fmt.Errorf("error generating feature set from custom features: %w", err)
531+
}
532+
533+
fg, err := featuregates.FeatureGateFromFeatureSets(configv1.FeatureSets, c.FeatureSet, customFS)
534+
if err != nil {
535+
return nil, fmt.Errorf("error generating feature gates from feature sets: %w", err)
536+
}
537+
return fg, nil
538+
}

‎pkg/types/validation/featuregate_test.go‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
package validation
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
8+
v1 "github.com/openshift/api/config/v1"
9+
"github.com/openshift/installer/pkg/types"
10+
"github.com/openshift/installer/pkg/types/gcp"
11+
"github.com/openshift/installer/pkg/types/vsphere"
12+
)
13+
14+
func TestFeatureGates(t *testing.T) {
15+
cases := []struct {
16+
name string
17+
installConfig *types.InstallConfig
18+
expected string
19+
}{
20+
{
21+
name: "GCP UserTags is allowed with Feature Gates enabled",
22+
installConfig: func() *types.InstallConfig {
23+
c := validInstallConfig()
24+
c.FeatureSet = v1.TechPreviewNoUpgrade
25+
c.GCP = validGCPPlatform()
26+
c.GCP.UserTags = []gcp.UserTag{{ParentID: "a", Key: "b", Value: "c"}}
27+
return c
28+
}(),
29+
},
30+
{
31+
name: "GCP UserTags is not allowed without Feature Gates",
32+
installConfig: func() *types.InstallConfig {
33+
c := validInstallConfig()
34+
c.GCP = validGCPPlatform()
35+
c.GCP.UserTags = []gcp.UserTag{{ParentID: "a", Key: "b", Value: "c"}}
36+
return c
37+
}(),
38+
expected: `^platform.gcp.userTags: Forbidden: this field is protected by the GCPLabelsTags feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set$`,
39+
},
40+
{
41+
name: "GCP UserLabels is allowed with Feature Gates enabled",
42+
installConfig: func() *types.InstallConfig {
43+
c := validInstallConfig()
44+
c.FeatureSet = v1.TechPreviewNoUpgrade
45+
c.GCP = validGCPPlatform()
46+
c.GCP.UserLabels = []gcp.UserLabel{{Key: "a", Value: "b"}}
47+
return c
48+
}(),
49+
},
50+
{
51+
name: "GCP UserLabels is not allowed without Feature Gates",
52+
installConfig: func() *types.InstallConfig {
53+
c := validInstallConfig()
54+
c.GCP = validGCPPlatform()
55+
c.GCP.UserLabels = []gcp.UserLabel{{Key: "a", Value: "b"}}
56+
return c
57+
}(),
58+
expected: `^platform.gcp.userLabels: Forbidden: this field is protected by the GCPLabelsTags feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set$`,
59+
},
60+
{
61+
name: "vSphere hosts is allowed with Feature Gates enabled",
62+
installConfig: func() *types.InstallConfig {
63+
c := validInstallConfig()
64+
c.FeatureSet = v1.TechPreviewNoUpgrade
65+
c.VSphere = validVSpherePlatform()
66+
c.VSphere.Hosts = []*vsphere.Host{{Role: "test"}}
67+
return c
68+
}(),
69+
},
70+
{
71+
name: "vSphere hosts is not allowed without Feature Gates",
72+
installConfig: func() *types.InstallConfig {
73+
c := validInstallConfig()
74+
c.VSphere = validVSpherePlatform()
75+
c.VSphere.Hosts = []*vsphere.Host{{Role: "test"}}
76+
return c
77+
}(),
78+
expected: `^platform.vsphere.hosts: Forbidden: this field is protected by the VSphereStaticIPs feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set$`,
79+
},
80+
{
81+
name: "vSphere hosts is allowed with custom Feature Gates",
82+
installConfig: func() *types.InstallConfig {
83+
c := validInstallConfig()
84+
c.FeatureSet = v1.CustomNoUpgrade
85+
c.FeatureGates = []string{"VSphereStaticIPs=true"}
86+
c.VSphere = validVSpherePlatform()
87+
c.VSphere.Hosts = []*vsphere.Host{{Role: "test"}}
88+
return c
89+
}(),
90+
},
91+
{
92+
name: "vSphere hosts is not allowed with custom Feature Gate disabled",
93+
installConfig: func() *types.InstallConfig {
94+
c := validInstallConfig()
95+
c.FeatureSet = v1.CustomNoUpgrade
96+
c.FeatureGates = []string{"VSphereStaticIPs=false"}
97+
c.VSphere = validVSpherePlatform()
98+
c.VSphere.Hosts = []*vsphere.Host{{Role: "test"}}
99+
return c
100+
}(),
101+
expected: `^platform.vsphere.hosts: Forbidden: this field is protected by the VSphereStaticIPs feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set$`,
102+
},
103+
}
104+
105+
for _, tc := range cases {
106+
t.Run(tc.name, func(t *testing.T) {
107+
err := ValidateFeatureSet(tc.installConfig).ToAggregate()
108+
if tc.expected == "" {
109+
assert.NoError(t, err)
110+
} else {
111+
assert.Regexp(t, tc.expected, err)
112+
}
113+
})
114+
}
115+
}

‎pkg/types/validation/installconfig.go‎

Lines changed: 53 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
azurevalidation "github.com/openshift/installer/pkg/types/azure/validation"
3333
"github.com/openshift/installer/pkg/types/baremetal"
3434
baremetalvalidation "github.com/openshift/installer/pkg/types/baremetal/validation"
35+
"github.com/openshift/installer/pkg/types/featuregates"
3536
"github.com/openshift/installer/pkg/types/gcp"
3637
gcpvalidation "github.com/openshift/installer/pkg/types/gcp/validation"
3738
"github.com/openshift/installer/pkg/types/ibmcloud"
@@ -169,7 +170,7 @@ func ValidateInstallConfig(c *types.InstallConfig, usingAgentMethod bool) field.
169170
}
170171
}
171172

172-
allErrs = append(allErrs, validateFeatureSet(c)...)
173+
allErrs = append(allErrs, ValidateFeatureSet(c)...)
173174

174175
return allErrs
175176
}
@@ -1047,8 +1048,8 @@ func validateAdditionalCABundlePolicy(c *types.InstallConfig) error {
10471048
}
10481049
}
10491050

1050-
// validateFeatureSet returns an error if a gated feature is used without opting into the feature set.
1051-
func validateFeatureSet(c *types.InstallConfig) field.ErrorList {
1051+
// ValidateFeatureSet returns an error if a gated feature is used without opting into the feature set.
1052+
func ValidateFeatureSet(c *types.InstallConfig) field.ErrorList {
10521053
allErrs := field.ErrorList{}
10531054

10541055
if _, ok := configv1.FeatureSets[c.FeatureSet]; !ok {
@@ -1070,29 +1071,10 @@ func validateFeatureSet(c *types.InstallConfig) field.ErrorList {
10701071
allErrs = append(allErrs, validateCustomFeatureGates(c)...)
10711072
}
10721073

1073-
if c.FeatureSet != configv1.TechPreviewNoUpgrade {
1074-
errMsg := "the TechPreviewNoUpgrade feature set must be enabled to use this field"
1075-
1076-
if c.OpenStack != nil {
1077-
for _, f := range openstackvalidation.FilledInTechPreviewFields(c) {
1078-
allErrs = append(allErrs, field.Forbidden(f, errMsg))
1079-
}
1080-
}
1081-
1082-
if c.VSphere != nil {
1083-
if len(c.VSphere.Hosts) > 0 {
1084-
allErrs = append(allErrs, field.Forbidden(field.NewPath("platform", "vsphere", "hosts"), errMsg))
1085-
}
1086-
}
1087-
1088-
if c.GCP != nil {
1089-
if len(c.GCP.UserTags) > 0 {
1090-
allErrs = append(allErrs, field.Forbidden(field.NewPath("platform", "gcp", "userTags"), errMsg))
1091-
}
1092-
if len(c.GCP.UserLabels) > 0 {
1093-
allErrs = append(allErrs, field.Forbidden(field.NewPath("platform", "gcp", "userLabels"), errMsg))
1094-
}
1095-
}
1074+
// We can only accurately check gated features
1075+
// if feature sets are correctly configured.
1076+
if len(allErrs) == 0 {
1077+
allErrs = append(allErrs, validateGatedFeatures(c)...)
10961078
}
10971079

10981080
return allErrs
@@ -1117,3 +1099,48 @@ func validateCustomFeatureGates(c *types.InstallConfig) field.ErrorList {
11171099

11181100
return allErrs
11191101
}
1102+
1103+
// validateGatedFeatures ensures that any gated features used in
1104+
// the install config are enabled.
1105+
func validateGatedFeatures(c *types.InstallConfig) field.ErrorList {
1106+
allErrs := field.ErrorList{}
1107+
1108+
// legacy feature gating
1109+
if c.FeatureSet != configv1.TechPreviewNoUpgrade {
1110+
errMsg := "the TechPreviewNoUpgrade feature set must be enabled to use this field"
1111+
1112+
if c.OpenStack != nil {
1113+
for _, f := range openstackvalidation.FilledInTechPreviewFields(c) {
1114+
allErrs = append(allErrs, field.Forbidden(f, errMsg))
1115+
}
1116+
}
1117+
}
1118+
1119+
gatedFeatures := []featuregates.GatedInstallConfigFeature{}
1120+
switch {
1121+
case c.GCP != nil:
1122+
gatedFeatures = append(gatedFeatures, gcpvalidation.GatedFeatures(c)...)
1123+
case c.VSphere != nil:
1124+
gatedFeatures = append(gatedFeatures, vspherevalidation.GatedFeatures(c)...)
1125+
}
1126+
1127+
fg, err := c.EnabledFeatureGates()
1128+
if err != nil {
1129+
errMsg := fmt.Errorf("error getting feature gates: %w", err)
1130+
return append(allErrs, field.InternalError(field.NewPath("featureSets"), errMsg))
1131+
}
1132+
1133+
errMsgTemplate := "this field is protected by the %s feature gate which must be enabled through either the TechPreviewNoUpgrade or CustomNoUpgrade feature set"
1134+
fgCheck := func(c featuregates.GatedInstallConfigFeature) {
1135+
if !fg.Enabled(c.FeatureGateName) && c.Condition {
1136+
errMsg := fmt.Sprintf(errMsgTemplate, c.FeatureGateName)
1137+
allErrs = append(allErrs, field.Forbidden(c.Field, errMsg))
1138+
}
1139+
}
1140+
1141+
for _, gf := range gatedFeatures {
1142+
fgCheck(gf)
1143+
}
1144+
1145+
return allErrs
1146+
}

‎pkg/types/vsphere/validation/featuregates.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package validation
2+
3+
import (
4+
"k8s.io/apimachinery/pkg/util/validation/field"
5+
6+
configv1 "github.com/openshift/api/config/v1"
7+
"github.com/openshift/installer/pkg/types"
8+
"github.com/openshift/installer/pkg/types/featuregates"
9+
)
10+
11+
// GatedFeatures determines all of the OpenStack install config fields that should
12+
// be validated to ensure that the proper featuregate is enabled when the field is used.
13+
func GatedFeatures(c *types.InstallConfig) []featuregates.GatedInstallConfigFeature {
14+
v := c.VSphere
15+
return []featuregates.GatedInstallConfigFeature{
16+
{
17+
FeatureGateName: configv1.FeatureGateVSphereStaticIPs,
18+
Condition: len(v.Hosts) > 0,
19+
Field: field.NewPath("platform", "vsphere", "hosts"),
20+
},
21+
}
22+
}

0 commit comments

Comments
 (0)