Skip to content

Commit

Permalink
Avoid using sets when existing slices suffice
Browse files Browse the repository at this point in the history
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 <skitt@redhat.com>
  • Loading branch information
skitt committed Jun 24, 2024
1 parent eb2f24c commit 7cb9d39
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 28 deletions.
4 changes: 2 additions & 2 deletions pkg/config/validation/general.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import (
"net"
"net/url"
"os"
"slices"
"strings"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"
)

Expand Down Expand Up @@ -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)
}
1 change: 1 addition & 0 deletions pkg/controller/factory/controller_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion pkg/oauth/tokenrequest/request_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net"
"net/http"
"net/url"
"slices"
"strings"

"github.com/RangelReale/osincli"
Expand Down Expand Up @@ -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
}
Expand Down
22 changes: 11 additions & 11 deletions pkg/operator/configobserver/featuregates/featuregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -17,32 +17,32 @@ 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
}

panic(fmt.Errorf("feature %q is not registered in FeatureGates %v", key, f.KnownFeatures()))
}

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
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package featuregates
import (
"errors"
"reflect"
"slices"
"testing"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -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)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/operator/encryption/controllers/prune_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package controllers

import (
"context"
"slices"
"sort"
"time"

Expand All @@ -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"

Expand Down Expand Up @@ -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)
Expand Down
7 changes: 2 additions & 5 deletions pkg/operator/staticpod/prune/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"path"
"path/filepath"
"slices"
"strconv"
"strings"
"time"
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
9 changes: 4 additions & 5 deletions pkg/route/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"regexp"
"slices"
"strings"

authorizationv1 "k8s.io/api/authorization/v1"
Expand Down Expand Up @@ -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.
Expand All @@ -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)
}

Expand All @@ -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, ", "))
)

Expand All @@ -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):
Expand Down

0 comments on commit 7cb9d39

Please sign in to comment.