Skip to content

Commit

Permalink
chore: Migrate to sets.Set[string] when possible
Browse files Browse the repository at this point in the history
  • Loading branch information
@pastequo
pastequo committed Jul 13, 2023
1 parent acf343f commit a779c2c
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 92 deletions.
20 changes: 10 additions & 10 deletions pkg/reconciler/ingress/ingress.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
ing.Status.InitializeConditions()
logger.Infof("Reconciling ingress: %#v", ing)

gatewayNames := map[v1alpha1.IngressVisibility]sets.String{}
gatewayNames := map[v1alpha1.IngressVisibility]sets.Set[string]{}
gatewayNames[v1alpha1.IngressVisibilityClusterLocal] = qualifiedGatewayNamesFromContext(ctx)[v1alpha1.IngressVisibilityClusterLocal]
gatewayNames[v1alpha1.IngressVisibilityExternalIP] = sets.String{}
gatewayNames[v1alpha1.IngressVisibilityExternalIP] = sets.New[string]()

ingressGateways := []*v1beta1.Gateway{}
if shouldReconcileTLS(ing) {
Expand Down Expand Up @@ -178,7 +178,7 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
// Otherwise, we fall back to the default global Gateways for HTTP behavior.
// We need this for the backward compatibility.
defaultGlobalHTTPGateways := qualifiedGatewayNamesFromContext(ctx)[v1alpha1.IngressVisibilityExternalIP]
gatewayNames[v1alpha1.IngressVisibilityExternalIP].Insert(defaultGlobalHTTPGateways.List()...)
gatewayNames[v1alpha1.IngressVisibilityExternalIP].Insert(sets.List(defaultGlobalHTTPGateways)...)
}

if err := r.reconcileIngressGateways(ctx, ingressGateways); err != nil {
Expand Down Expand Up @@ -235,13 +235,13 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
}

func getPublicHosts(ing *v1alpha1.Ingress) []string {
hosts := sets.String{}
hosts := sets.New[string]()
for _, rule := range ing.Spec.Rules {
if rule.Visibility == v1alpha1.IngressVisibilityExternalIP {
hosts.Insert(rule.Hosts...)
}
}
return hosts.List()
return sets.List(hosts)
}

func (r *Reconciler) reconcileCertSecrets(ctx context.Context, ing *v1alpha1.Ingress, desiredSecrets []*corev1.Secret) error {
Expand Down Expand Up @@ -297,7 +297,7 @@ func (r *Reconciler) reconcileSystemGeneratedGateway(ctx context.Context, desire
func (r *Reconciler) reconcileVirtualServices(ctx context.Context, ing *v1alpha1.Ingress,
desired []*v1beta1.VirtualService) error {
// First, create all needed VirtualServices.
kept := sets.NewString()
kept := sets.New[string]()
for _, d := range desired {
if d.GetAnnotations()[networking.IngressClassAnnotationKey] != netconfig.IstioIngressClassName {
// We do not create resources that do not have istio ingress class annotation.
Expand Down Expand Up @@ -438,19 +438,19 @@ func (r *Reconciler) GetVirtualServiceLister() istiolisters.VirtualServiceLister
}

// qualifiedGatewayNamesFromContext get gateway names from context
func qualifiedGatewayNamesFromContext(ctx context.Context) map[v1alpha1.IngressVisibility]sets.String {
func qualifiedGatewayNamesFromContext(ctx context.Context) map[v1alpha1.IngressVisibility]sets.Set[string] {
ci := config.FromContext(ctx).Istio
publicGateways := make(sets.String, len(ci.IngressGateways))
publicGateways := sets.New[string]()
for _, gw := range ci.IngressGateways {
publicGateways.Insert(gw.QualifiedName())
}

privateGateways := make(sets.String, len(ci.LocalGateways))
privateGateways := sets.New[string]()
for _, gw := range ci.LocalGateways {
privateGateways.Insert(gw.QualifiedName())
}

return map[v1alpha1.IngressVisibility]sets.String{
return map[v1alpha1.IngressVisibility]sets.Set[string]{
v1alpha1.IngressVisibilityExternalIP: publicGateways,
v1alpha1.IngressVisibilityClusterLocal: privateGateways,
}
Expand Down
20 changes: 10 additions & 10 deletions pkg/reconciler/ingress/ingress_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,11 @@ var (
"gateway." + config.KnativeIngressGateway: newDomainInternal,
"gateway.knative-test-gateway": originDomainInternal,
}
ingressGateway = map[v1alpha1.IngressVisibility]sets.String{
v1alpha1.IngressVisibilityExternalIP: sets.NewString(config.KnativeIngressGateway),
ingressGateway = map[v1alpha1.IngressVisibility]sets.Set[string]{
v1alpha1.IngressVisibilityExternalIP: sets.New(config.KnativeIngressGateway),
}
gateways = map[v1alpha1.IngressVisibility]sets.String{
v1alpha1.IngressVisibilityExternalIP: sets.NewString("knative-test-gateway", config.KnativeIngressGateway),
gateways = map[v1alpha1.IngressVisibility]sets.Set[string]{
v1alpha1.IngressVisibilityExternalIP: sets.New("knative-test-gateway", config.KnativeIngressGateway),
}
perIngressGatewayName = resources.GatewayName(ingressWithTLS("reconciling-ingress", ingressTLS), ingressService)
)
Expand Down Expand Up @@ -1256,7 +1256,7 @@ func ingressWithTLSAndStatusClusterLocal(name string, tls []v1alpha1.IngressTLS,
return ci
}

func meshVirtualServiceWithStatus(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.String, status *istiov1alpha1.IstioStatus, generation int64, observedGeneration int64) *v1beta1.VirtualService {
func meshVirtualServiceWithStatus(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string], status *istiov1alpha1.IstioStatus, generation int64, observedGeneration int64) *v1beta1.VirtualService {

Check failure on line 1259 in pkg/reconciler/ingress/ingress_test.go

View workflow job for this annotation

GitHub Actions / style / Golang / Lint 

func `meshVirtualServiceWithStatus` is unused (unused)
vs := resources.MakeMeshVirtualService(ing, gateways)
vs.Status = *status.DeepCopy()
vs.ObjectMeta.Generation = generation
Expand All @@ -1265,7 +1265,7 @@ func meshVirtualServiceWithStatus(ing *v1alpha1.Ingress, gateways map[v1alpha1.I
return vs
}

func ingressVirtualServiceWithStatus(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.String, status *istiov1alpha1.IstioStatus, generation int64, observedGeneration int64) *v1beta1.VirtualService {
func ingressVirtualServiceWithStatus(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string], status *istiov1alpha1.IstioStatus, generation int64, observedGeneration int64) *v1beta1.VirtualService {

Check failure on line 1268 in pkg/reconciler/ingress/ingress_test.go

View workflow job for this annotation

GitHub Actions / style / Golang / Lint 

func `ingressVirtualServiceWithStatus` is unused (unused)
vs := resources.MakeIngressVirtualService(ing, gateways)
vs.Status = *status.DeepCopy()
vs.ObjectMeta.Generation = generation
Expand Down Expand Up @@ -1536,10 +1536,10 @@ func TestGlobalResyncOnUpdateNetwork(t *testing.T) {
}
}

func makeGatewayMap(publicGateways []string, privateGateways []string) map[v1alpha1.IngressVisibility]sets.String {
return map[v1alpha1.IngressVisibility]sets.String{
v1alpha1.IngressVisibilityExternalIP: sets.NewString(publicGateways...),
v1alpha1.IngressVisibilityClusterLocal: sets.NewString(privateGateways...),
func makeGatewayMap(publicGateways []string, privateGateways []string) map[v1alpha1.IngressVisibility]sets.Set[string] {
return map[v1alpha1.IngressVisibility]sets.Set[string]{
v1alpha1.IngressVisibilityExternalIP: sets.New(publicGateways...),
v1alpha1.IngressVisibilityClusterLocal: sets.New(privateGateways...),
}
}

Expand Down
12 changes: 10 additions & 2 deletions pkg/reconciler/ingress/lister.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type gatewayPodTargetLister struct {

func (l *gatewayPodTargetLister) ListProbeTargets(ctx context.Context, ing *v1alpha1.Ingress) ([]status.ProbeTarget, error) {
results := []status.ProbeTarget{}
hostsByGateway := ingress.HostsPerVisibility(ing, qualifiedGatewayNamesFromContext(ctx))
hostsByGateway := ingress.HostsPerVisibility(ing, convertVisibilityMap(qualifiedGatewayNamesFromContext(ctx)))
gatewayNames := make([]string, 0, len(hostsByGateway))
for gatewayName := range hostsByGateway {
gatewayNames = append(gatewayNames, gatewayName)
Expand Down Expand Up @@ -129,7 +129,7 @@ func (l *gatewayPodTargetLister) listGatewayTargets(gateway *v1beta1.Gateway) ([
return nil, fmt.Errorf("failed to get Endpoints: %w", err)
}

seen := sets.NewString()
seen := sets.New[string]()
targets := []status.ProbeTarget{}
for _, server := range gateway.Spec.Servers {
tURL := &url.URL{}
Expand Down Expand Up @@ -187,3 +187,11 @@ func (l *gatewayPodTargetLister) listGatewayTargets(gateway *v1beta1.Gateway) ([
}
return targets, nil
}

// Remove me when ingress.HostsPerVisibility uses sets.Set[string]
func convertVisibilityMap(input map[v1alpha1.IngressVisibility]sets.Set[string]) map[v1alpha1.IngressVisibility]sets.String {

Check failure on line 192 in pkg/reconciler/ingress/lister.go

View workflow job for this annotation

GitHub Actions / style / Golang / Lint 

SA1019: sets.String is deprecated: use generic Set instead. new ways: s1 := Set[string]{} s2 := New[string]()  (staticcheck)
return map[v1alpha1.IngressVisibility]sets.String{

Check failure on line 193 in pkg/reconciler/ingress/lister.go

View workflow job for this annotation

GitHub Actions / style / Golang / Lint 

SA1019: sets.String is deprecated: use generic Set instead. new ways: s1 := Set[string]{} s2 := New[string]()  (staticcheck)
v1alpha1.IngressVisibilityExternalIP: sets.NewString(sets.List(input[v1alpha1.IngressVisibilityExternalIP])...),
v1alpha1.IngressVisibilityClusterLocal: sets.NewString(sets.List(input[v1alpha1.IngressVisibilityClusterLocal])...),
}
}
2 changes: 1 addition & 1 deletion pkg/reconciler/ingress/resources/gateway.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func GetIngressGatewaySvcNameNamespaces(ctx context.Context) ([]metav1.ObjectMet

// UpdateGateway replaces the existing servers with the wanted servers.
func UpdateGateway(gateway *v1beta1.Gateway, want []*istiov1beta1.Server, existing []*istiov1beta1.Server) *v1beta1.Gateway {
existingServers := sets.String{}
existingServers := sets.New[string]()
for i := range existing {
existingServers.Insert(existing[i].Port.Name)
}
Expand Down
61 changes: 35 additions & 26 deletions pkg/reconciler/ingress/resources/virtual_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ func VirtualServiceNamespace(ing *v1alpha1.Ingress) string {

// MakeIngressVirtualService creates Istio VirtualService as network
// programming for Istio Gateways other than 'mesh'.
func MakeIngressVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.String) *v1beta1.VirtualService {
func MakeIngressVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string]) *v1beta1.VirtualService {
vs := &v1beta1.VirtualService{
ObjectMeta: metav1.ObjectMeta{
Name: names.IngressVirtualService(ing),
Namespace: VirtualServiceNamespace(ing),
OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)},
Annotations: ing.GetAnnotations(),
},
Spec: *makeVirtualServiceSpec(ing, gateways, ingress.ExpandedHosts(getHosts(ing))),
Spec: *makeVirtualServiceSpec(ing, gateways, expandedHosts(getHosts(ing))),
}

// Populate the Ingress labels.
Expand All @@ -65,12 +65,12 @@ func MakeIngressVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.Ingr
}

// MakeMeshVirtualService creates a mesh Virtual Service
func MakeMeshVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.String) *v1beta1.VirtualService {
func MakeMeshVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string]) *v1beta1.VirtualService {
hosts := keepLocalHostnames(getHosts(ing))
// If cluster local gateway is configured, we need to expand hosts because of
// https://github.com/knative/serving/issues/6488#issuecomment-573513768.
if len(gateways[v1alpha1.IngressVisibilityClusterLocal]) != 0 {
hosts = ingress.ExpandedHosts(hosts)
hosts = expandedHosts(hosts)
}
if len(hosts) == 0 {
return nil
Expand All @@ -82,9 +82,9 @@ func MakeMeshVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.Ingress
OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(ing)},
Annotations: ing.GetAnnotations(),
},
Spec: *makeVirtualServiceSpec(ing, map[v1alpha1.IngressVisibility]sets.String{
v1alpha1.IngressVisibilityExternalIP: sets.NewString("mesh"),
v1alpha1.IngressVisibilityClusterLocal: sets.NewString("mesh"),
Spec: *makeVirtualServiceSpec(ing, map[v1alpha1.IngressVisibility]sets.Set[string]{
v1alpha1.IngressVisibilityExternalIP: sets.New("mesh"),
v1alpha1.IngressVisibilityClusterLocal: sets.New("mesh"),
}, hosts),
}
// Populate the Ingress labels.
Expand All @@ -96,7 +96,7 @@ func MakeMeshVirtualService(ing *v1alpha1.Ingress, gateways map[v1alpha1.Ingress
}

// MakeVirtualServices creates a mesh VirtualService and a virtual service for each gateway
func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.String) ([]*v1beta1.VirtualService, error) {
func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string]) ([]*v1beta1.VirtualService, error) {
// Insert probe header
ing = ing.DeepCopy()
if _, err := ingress.InsertProbe(ing); err != nil {
Expand All @@ -122,39 +122,39 @@ func MakeVirtualServices(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVis
return vss, nil
}

func makeVirtualServiceSpec(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.String, hosts sets.String) *istiov1beta1.VirtualService {
func makeVirtualServiceSpec(ing *v1alpha1.Ingress, gateways map[v1alpha1.IngressVisibility]sets.Set[string], hosts sets.Set[string]) *istiov1beta1.VirtualService {
spec := istiov1beta1.VirtualService{
Hosts: hosts.List(),
Hosts: sets.List(hosts),
}

gw := sets.String{}
gw := sets.New[string]()
for _, rule := range ing.Spec.Rules {
for i := range rule.HTTP.Paths {
p := rule.HTTP.Paths[i]
hosts := hosts.Intersection(sets.NewString(rule.Hosts...))
hosts := hosts.Intersection(sets.New(rule.Hosts...))
if hosts.Len() != 0 {
http := makeVirtualServiceRoute(hosts, &p, gateways, rule.Visibility)
// Add all the Gateways that exist inside the http.match section of
// the VirtualService.
// This ensures that we are only using the Gateways that actually appear
// in VirtualService routes.
for _, m := range http.Match {
gw = gw.Union(sets.NewString(m.Gateways...))
gw = gw.Union(sets.New(m.Gateways...))
}
spec.Http = append(spec.Http, http)
}
}
}
spec.Gateways = gw.List()
spec.Gateways = sets.List(gw)
return &spec
}

func makeVirtualServiceRoute(hosts sets.String, http *v1alpha1.HTTPIngressPath, gateways map[v1alpha1.IngressVisibility]sets.String, visibility v1alpha1.IngressVisibility) *istiov1beta1.HTTPRoute {
func makeVirtualServiceRoute(hosts sets.Set[string], http *v1alpha1.HTTPIngressPath, gateways map[v1alpha1.IngressVisibility]sets.Set[string], visibility v1alpha1.IngressVisibility) *istiov1beta1.HTTPRoute {
matches := []*istiov1beta1.HTTPMatchRequest{}
// Deduplicate hosts to avoid excessive matches, which cause a combinatorial expansion in Istio
distinctHosts := getDistinctHostPrefixes(hosts)

for _, host := range distinctHosts.List() {
for _, host := range sets.List(distinctHosts) {
matches = append(matches, makeMatch(host, http.Path, http.Headers, gateways[visibility]))
}

Expand Down Expand Up @@ -210,11 +210,11 @@ func makeVirtualServiceRoute(hosts sets.String, http *v1alpha1.HTTPIngressPath,

// getDistinctHostPrefixes deduplicate a set of prefix matches. For example, the set {a, aabb} can be
// reduced to {a}, as a prefix match on {a} accepts all the same inputs as {a, aabb}.
func getDistinctHostPrefixes(hosts sets.String) sets.String {
func getDistinctHostPrefixes(hosts sets.Set[string]) sets.Set[string] {
// First we sort the list. This ensures that we always process the smallest elements (which match against
// the most patterns, as they are less specific) first.
all := hosts.List()
ns := sets.NewString()
all := sets.List(hosts)
ns := sets.New[string]()
for _, h := range all {
prefixExists := false
h = hostPrefix(h)
Expand All @@ -233,20 +233,20 @@ func getDistinctHostPrefixes(hosts sets.String) sets.String {
return ns
}

func keepLocalHostnames(hosts sets.String) sets.String {
func keepLocalHostnames(hosts sets.Set[string]) sets.Set[string] {
localSvcSuffix := ".svc." + network.GetClusterDomainName()
retained := sets.NewString()
for _, h := range hosts.List() {
retained := sets.New[string]()
for _, h := range sets.List(hosts) {
if strings.HasSuffix(h, localSvcSuffix) {
retained.Insert(h)
}
}
return retained
}

func makeMatch(host, path string, headers map[string]v1alpha1.HeaderMatch, gateways sets.String) *istiov1beta1.HTTPMatchRequest {
func makeMatch(host, path string, headers map[string]v1alpha1.HeaderMatch, gateways sets.Set[string]) *istiov1beta1.HTTPMatchRequest {
match := &istiov1beta1.HTTPMatchRequest{
Gateways: gateways.List(),
Gateways: sets.List(gateways),
Authority: &istiov1beta1.StringMatch{
// Do not use Regex as Istio 1.4 or later has 100 bytes limitation.
MatchType: &istiov1beta1.StringMatch_Prefix{Prefix: host},
Expand Down Expand Up @@ -283,8 +283,8 @@ func hostPrefix(host string) string {
return strings.TrimSuffix(host, localDomainSuffix)
}

func getHosts(ing *v1alpha1.Ingress) sets.String {
hosts := sets.NewString()
func getHosts(ing *v1alpha1.Ingress) sets.Set[string] {
hosts := sets.New[string]()
for _, rule := range ing.Spec.Rules {
hosts.Insert(rule.Hosts...)
}
Expand Down Expand Up @@ -312,3 +312,12 @@ func getPublicIngressRules(i *v1alpha1.Ingress) []v1alpha1.IngressRule {

return result
}

// Keep me until ingress.ExpandedHosts uses sets.Set[string]
func expandedHosts(hosts sets.Set[string]) sets.Set[string] {
tmp := sets.NewString(sets.List(hosts)...)

ret := ingress.ExpandedHosts(tmp)

return sets.New(ret.List()...)
}
Loading

0 comments on commit a779c2c

Please sign in to comment.