Skip to content

OCPBUGS-73946: Ignore (Dev|Tech)PreviewNoUpgrade FeatureSets #6082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copejon wants to merge 3 commits into
base: main
Choose a base branch
from
Open

OCPBUGS-73946: Ignore (Dev|Tech)PreviewNoUpgrade FeatureSets #6082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1910b01
USHIFT-6487: Set aws machine name limit to 64
pacevedom Jan 16, 2026
81654ae
do not allow users to set Dev/TechPreview feature sets
copejon Jan 19, 2026
7cfe84c
Apply suggestion from @agullon
copejon Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 20 additions & 31 deletions pkg/config/apiserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"slices"

configv1 "github.com/openshift/api/config/v1"
featuresUtils "github.com/openshift/api/features"
"github.com/openshift/library-go/pkg/crypto"

"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -154,28 +153,15 @@ type FeatureGates struct {
CustomNoUpgrade CustomNoUpgrade `json:"customNoUpgrade"`
}

// ToApiserverArgs converts the FeatureGates struct to a list of feature-gates arguments for the kube-apiserver.
// Validation checks should be performed before calling this function to ensure the FeatureGates struct is valid.
func (fg FeatureGates) ToApiserverArgs() ([]string, error) {
ret := sets.NewString()

switch fg.FeatureSet {
case FeatureSetCustomNoUpgrade:
for _, feature := range fg.CustomNoUpgrade.Enabled {
ret.Insert(fmt.Sprintf("%s=true", feature))
}
for _, feature := range fg.CustomNoUpgrade.Disabled {
ret.Insert(fmt.Sprintf("%s=false", feature))
}
case FeatureSetDevPreviewNoUpgrade, FeatureSetTechPreviewNoUpgrade:
fgEnabledDisabled, err := featuresUtils.FeatureSets(featuresUtils.SelfManaged, configv1.FeatureSet(fg.FeatureSet))
if err != nil {
return nil, fmt.Errorf("failed to get feature set gates: %w", err)
}
for _, f := range fgEnabledDisabled.Enabled {
ret.Insert(fmt.Sprintf("%s=true", f.FeatureGateAttributes.Name))
}
for _, f := range fgEnabledDisabled.Disabled {
ret.Insert(fmt.Sprintf("%s=false", f.FeatureGateAttributes.Name))
}
for _, feature := range fg.CustomNoUpgrade.Enabled {
ret.Insert(fmt.Sprintf("%s=true", feature))
}
for _, feature := range fg.CustomNoUpgrade.Disabled {
ret.Insert(fmt.Sprintf("%s=false", feature))
}
return ret.List(), nil
}
Expand All @@ -190,18 +176,21 @@ func (fg *FeatureGates) validateFeatureGates() error {
if fg == nil || reflect.DeepEqual(*fg, FeatureGates{}) {
return nil
}
// Must use a recognized feature set, or else empty
if fg.FeatureSet != "" && fg.FeatureSet != FeatureSetCustomNoUpgrade && fg.FeatureSet != FeatureSetTechPreviewNoUpgrade && fg.FeatureSet != FeatureSetDevPreviewNoUpgrade {

// FeatureSet must be empty or CustomNoUpgrade. If empty, CustomNoUpgrade.Enabled/Disabled lists must be empty.
switch fg.FeatureSet {
case "":
if len(fg.CustomNoUpgrade.Enabled) > 0 || len(fg.CustomNoUpgrade.Disabled) > 0 {
return fmt.Errorf("CustomNoUpgrade enabled/disabled lists must be empty when FeatureSet is empty")
}
return nil
case FeatureSetCustomNoUpgrade:
// Valid - continue to validate enabled/disabled lists below
case FeatureSetDevPreviewNoUpgrade, FeatureSetTechPreviewNoUpgrade:
return fmt.Errorf("FeatureSet %s is not supported. Use CustomNoUpgrade to enable/disable feature gates", fg.FeatureSet)
default:
return fmt.Errorf("invalid feature set: %s", fg.FeatureSet)
}
// Must set FeatureSet to CustomNoUpgrade to use custom feature gates
if fg.FeatureSet != FeatureSetCustomNoUpgrade && (len(fg.CustomNoUpgrade.Enabled) > 0 || len(fg.CustomNoUpgrade.Disabled) > 0) {
return fmt.Errorf("CustomNoUpgrade must be empty when FeatureSet is empty")
}
// Must set CustomNoUpgrade enabled or disabled lists when FeatureSet is CustomNoUpgrade
if fg.FeatureSet == FeatureSetCustomNoUpgrade && len(fg.CustomNoUpgrade.Enabled) == 0 && len(fg.CustomNoUpgrade.Disabled) == 0 {
return fmt.Errorf("CustomNoUpgrade enabled or disabled lists must be set when FeatureSet is CustomNoUpgrade")
}

var errs = make(sets.Set[error], 0)
for _, requiredFG := range RequiredFeatureGates {
Expand Down
44 changes: 38 additions & 6 deletions pkg/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -819,14 +819,15 @@ func TestValidate(t *testing.T) {
expectErr: true,
},
{
name: "feature-gates-custom-no-upgrade-with-feature-set",
name: "feature-gates-custom-no-upgrade-valid",
config: func() *Config {
c := mkDefaultConfig()
c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade"
c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"}
c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature2"}
return c
}(),
expectErr: false,
},
{
name: "feature-gates-custom-no-upgrade-with-feature-set-empty",
Expand All @@ -840,23 +841,54 @@ func TestValidate(t *testing.T) {
expectErr: true,
},
{
name: "feature-gates-custom-no-upgrade-with-empty-enabled-and-disabled-lists",
name: "feature-gates-custom-no-upgrade-enabled-and-disabled-have-same-feature-gate",
config: func() *Config {
c := mkDefaultConfig()
c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade"
c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{}
c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{}
c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"}
c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature1"}
return c
}(),
expectErr: true,
},
{
name: "feature-gates-custom-no-upgrade-enabled-and-disabled-have-same-feature-gate",
name: "feature-gates-required-feature-gate-cannot-be-disabled",
config: func() *Config {
c := mkDefaultConfig()
c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade"
c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"feature1"}
c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature1"}
c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"UserNamespacesSupport"}
return c
}(),
expectErr: true,
},
{
name: "feature-gates-required-feature-gate-cannot-be-explicitly-enabled",
config: func() *Config {
c := mkDefaultConfig()
c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade"
c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{"UserNamespacesSupport"}
c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{"feature2"}
return c
}(),
expectErr: true,
},
{
name: "feature-gates-custom-no-upgrade-with-empty-enabled-and-disabled-lists",
config: func() *Config {
c := mkDefaultConfig()
c.ApiServer.FeatureGates.FeatureSet = "CustomNoUpgrade"
c.ApiServer.FeatureGates.CustomNoUpgrade.Enabled = []string{}
c.ApiServer.FeatureGates.CustomNoUpgrade.Disabled = []string{}
return c
}(),
expectErr: false,
},
{
name: "feature-gates-preview-feature-sets-not-supported",
config: func() *Config {
c := mkDefaultConfig()
c.ApiServer.FeatureGates.FeatureSet = "TechPreviewNoUpgrade"
return c
}(),
expectErr: true,
Expand Down
4 changes: 2 additions & 2 deletions scripts/aws/cf-gen.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ Parameters:
Description: Current RHEL AMI to use.
Type: AWS::EC2::Image::Id
Machinename:
AllowedPattern: ^([a-zA-Z][a-zA-Z0-9\-]{0,26})$
MaxLength: 27
AllowedPattern: ^([a-zA-Z][a-zA-Z0-9\-]*)$
MaxLength: 64
MinLength: 1
ConstraintDescription: Machinename
Description: Machinename
Expand Down