From 7cb9d39185d886590f18cecb2643a903ff511a27 Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Mon, 15 Apr 2024 17:26:11 +0200 Subject: [PATCH] Avoid using sets when existing slices suffice Now that Go includes slice operations such as slices.Contains, some uses of sets are no longer useful. In particular, creating a set using a slice just to check whether the slice contains a specific item is more expensive than checking the slice directly. Signed-off-by: Stephen Kitt --- pkg/config/validation/general.go | 4 ++-- pkg/controller/factory/controller_context.go | 1 + pkg/oauth/tokenrequest/request_token.go | 3 ++- .../featuregates/featuregate.go | 22 +++++++++---------- .../featuregates/observe_featuregates_test.go | 3 +++ .../controllers/prune_controller.go | 8 +++---- pkg/operator/staticpod/prune/cmd.go | 7 ++---- pkg/route/validation/validation.go | 9 ++++---- 8 files changed, 29 insertions(+), 28 deletions(-) diff --git a/pkg/config/validation/general.go b/pkg/config/validation/general.go index 8319847605..bf47e7b551 100644 --- a/pkg/config/validation/general.go +++ b/pkg/config/validation/general.go @@ -5,9 +5,9 @@ import ( "net" "net/url" "os" + "slices" "strings" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -126,5 +126,5 @@ func HostnameMatchSpecCandidates(hostname string) []string { // HostnameMatches returns true if the given hostname is matched by the given matchSpec func HostnameMatches(hostname string, matchSpec string) bool { - return sets.New(HostnameMatchSpecCandidates(hostname)...).Has(matchSpec) + return slices.Contains(HostnameMatchSpecCandidates(hostname), matchSpec) } diff --git a/pkg/controller/factory/controller_context.go b/pkg/controller/factory/controller_context.go index 15b8bdf706..88436c9f10 100644 --- a/pkg/controller/factory/controller_context.go +++ b/pkg/controller/factory/controller_context.go @@ -96,6 +96,7 @@ func (c syncContext) enqueueKeys(keys ...string) { // (or its tombstone) is a namespace and it matches a name of any namespaces // that we are interested in func namespaceChecker(interestingNamespaces []string) func(obj interface{}) bool { + // This is used for quick lookups in informers interestingNamespacesSet := sets.New(interestingNamespaces...) return func(obj interface{}) bool { diff --git a/pkg/oauth/tokenrequest/request_token.go b/pkg/oauth/tokenrequest/request_token.go index 431624df9e..db1f7f4434 100644 --- a/pkg/oauth/tokenrequest/request_token.go +++ b/pkg/oauth/tokenrequest/request_token.go @@ -10,6 +10,7 @@ import ( "net" "net/http" "net/url" + "slices" "strings" "github.com/RangelReale/osincli" @@ -192,7 +193,7 @@ func (o *RequestTokenOptions) SetDefaultOsinConfig(clientID string, redirectURL config.RedirectUrl = *redirectURL } - if !o.TokenFlow && sets.New(metadata.CodeChallengeMethodsSupported...).Has(pkce_s256) { + if !o.TokenFlow && slices.Contains(metadata.CodeChallengeMethodsSupported, pkce_s256) { if err := osincli.PopulatePKCE(config); err != nil { return err } diff --git a/pkg/operator/configobserver/featuregates/featuregate.go b/pkg/operator/configobserver/featuregates/featuregate.go index 33275e0ec0..5792ada3a5 100644 --- a/pkg/operator/configobserver/featuregates/featuregate.go +++ b/pkg/operator/configobserver/featuregates/featuregate.go @@ -2,9 +2,9 @@ package featuregates import ( "fmt" + "slices" configv1 "github.com/openshift/api/config/v1" - "k8s.io/apimachinery/pkg/util/sets" ) // FeatureGate indicates whether a given feature is enabled or not @@ -17,22 +17,22 @@ type FeatureGate interface { } type featureGate struct { - enabled sets.Set[configv1.FeatureGateName] - disabled sets.Set[configv1.FeatureGateName] + enabled []configv1.FeatureGateName + disabled []configv1.FeatureGateName } func NewFeatureGate(enabled, disabled []configv1.FeatureGateName) FeatureGate { return &featureGate{ - enabled: sets.New[configv1.FeatureGateName](enabled...), - disabled: sets.New[configv1.FeatureGateName](disabled...), + enabled: enabled, + disabled: disabled, } } func (f *featureGate) Enabled(key configv1.FeatureGateName) bool { - if f.enabled.Has(key) { + if slices.Contains(f.enabled, key) { return true } - if f.disabled.Has(key) { + if slices.Contains(f.disabled, key) { return false } @@ -40,9 +40,9 @@ func (f *featureGate) Enabled(key configv1.FeatureGateName) bool { } func (f *featureGate) KnownFeatures() []configv1.FeatureGateName { - allKnown := sets.New[string]() - allKnown.Insert(FeatureGateNamesToStrings(f.enabled.UnsortedList())...) - allKnown.Insert(FeatureGateNamesToStrings(f.disabled.UnsortedList())...) + allKnown := make([]configv1.FeatureGateName, 0, len(f.enabled)+len(f.disabled)) + allKnown = append(allKnown, f.enabled...) + allKnown = append(allKnown, f.disabled...) - return StringsToFeatureGateNames(sets.List(allKnown)) + return allKnown } diff --git a/pkg/operator/configobserver/featuregates/observe_featuregates_test.go b/pkg/operator/configobserver/featuregates/observe_featuregates_test.go index 1242942802..2d0c6742f2 100644 --- a/pkg/operator/configobserver/featuregates/observe_featuregates_test.go +++ b/pkg/operator/configobserver/featuregates/observe_featuregates_test.go @@ -3,6 +3,7 @@ package featuregates import ( "errors" "reflect" + "slices" "testing" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -167,6 +168,8 @@ func TestObserveFeatureFlags(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + // Sort the result for comparison with the expected result + slices.Sort(actual) if !reflect.DeepEqual(tc.expectedResult, actual) { t.Errorf("Unexpected features gates\n got: %v\n expected: %v", actual, tc.expectedResult) } diff --git a/pkg/operator/encryption/controllers/prune_controller.go b/pkg/operator/encryption/controllers/prune_controller.go index e11282f50e..5042812c26 100644 --- a/pkg/operator/encryption/controllers/prune_controller.go +++ b/pkg/operator/encryption/controllers/prune_controller.go @@ -2,6 +2,7 @@ package controllers import ( "context" + "slices" "sort" "time" @@ -10,7 +11,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/apimachinery/pkg/util/sets" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/klog/v2" @@ -160,9 +160,9 @@ NextEncryptionSecret: // remove our finalizer if it is present secret := s.DeepCopy() - if finalizers := sets.New(secret.Finalizers...); finalizers.Has(secrets.EncryptionSecretFinalizer) { - delete(finalizers, secrets.EncryptionSecretFinalizer) - secret.Finalizers = sets.List(finalizers) + idx := slices.Index(secret.Finalizers, secrets.EncryptionSecretFinalizer) + if idx > -1 { + secret.Finalizers = slices.Delete(secret.Finalizers, idx, idx+1) var updateErr error secret, updateErr = c.secretClient.Secrets("openshift-config-managed").Update(ctx, secret, metav1.UpdateOptions{}) deleteErrs = append(deleteErrs, updateErr) diff --git a/pkg/operator/staticpod/prune/cmd.go b/pkg/operator/staticpod/prune/cmd.go index 6fbf7a06b8..b029b74619 100644 --- a/pkg/operator/staticpod/prune/cmd.go +++ b/pkg/operator/staticpod/prune/cmd.go @@ -5,6 +5,7 @@ import ( "os" "path" "path/filepath" + "slices" "strconv" "strings" "time" @@ -13,8 +14,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" "k8s.io/klog/v2" - - "k8s.io/apimachinery/pkg/util/sets" ) type PruneOptions struct { @@ -77,8 +76,6 @@ func (o *PruneOptions) Validate() error { } func (o *PruneOptions) Run() error { - protectedIDs := sets.New(o.ProtectedRevisions...) - files, err := os.ReadDir(o.ResourceDir) if err != nil { return err @@ -102,7 +99,7 @@ func (o *PruneOptions) Run() error { } // And is not protected... - if protected := protectedIDs.Has(revisionID); protected { + if protected := slices.Contains(o.ProtectedRevisions, revisionID); protected { continue } // And is less than or equal to the maxEligibleRevisionID diff --git a/pkg/route/validation/validation.go b/pkg/route/validation/validation.go index 020e82983e..ca4850bd26 100644 --- a/pkg/route/validation/validation.go +++ b/pkg/route/validation/validation.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "regexp" + "slices" "strings" authorizationv1 "k8s.io/api/authorization/v1" @@ -400,8 +401,7 @@ func validateInsecureEdgeTerminationPolicy(tls *routev1.TLSConfig, fldPath *fiel } var ( - allowedWildcardPolicies = []string{string(routev1.WildcardPolicyNone), string(routev1.WildcardPolicySubdomain)} - allowedWildcardPoliciesSet = sets.New(allowedWildcardPolicies...) + allowedWildcardPolicies = []routev1.WildcardPolicyType{routev1.WildcardPolicyNone, routev1.WildcardPolicySubdomain} ) // validateWildcardPolicy tests that the wildcard policy is either empty or one of the supported types. @@ -411,7 +411,7 @@ func validateWildcardPolicy(host string, policy routev1.WildcardPolicyType, fldP } // Check if policy is one of None or Subdomain. - if !allowedWildcardPoliciesSet.Has(string(policy)) { + if !slices.Contains(allowedWildcardPolicies, policy) { return field.NotSupported(fldPath, policy, allowedWildcardPolicies) } @@ -424,7 +424,6 @@ func validateWildcardPolicy(host string, policy routev1.WildcardPolicyType, fldP var ( notAllowedHTTPHeaders = []string{"strict-transport-security", "proxy", "cookie", "set-cookie"} - notAllowedHTTPHeaderSet = sets.New(notAllowedHTTPHeaders...) notAllowedHTTPHeadersMessage = fmt.Sprintf("the following headers may not be modified using this API: %v", strings.Join(notAllowedHTTPHeaders, ", ")) ) @@ -451,7 +450,7 @@ func validateHeaders(fldPath *field.Path, headers []routev1.RouteHTTPHeader, val case nameLength > maxHeaderNameSize: err := field.Invalid(idxPath.Child("name"), header.Name, fmt.Sprintf("name exceeds the maximum length, which is %d", maxHeaderNameSize)) allErrs = append(allErrs, err) - case notAllowedHTTPHeaderSet.Has(strings.ToLower(header.Name)): + case slices.Contains(notAllowedHTTPHeaders, strings.ToLower(header.Name)): err := field.Forbidden(idxPath.Child("name"), notAllowedHTTPHeadersMessage) allErrs = append(allErrs, err) case !permittedHeaderNameRE.MatchString(header.Name):