Skip to content

Commit

Permalink
internal/dag: stop embedding ServicePort in dag.Service (projectconto…
Browse files Browse the repository at this point in the history 
…ur#2785)

The Kubernetes `v1.ServicePort` is embedded in `dag.Service` as a pointer
field. There's no need to have a pointer here, since it is always assumed
to be non-nil, and using the type as an explicit field makes is easier to
see where it is used, and clarifies overlapping field (i.e. `Protocol`).

This updates projectcontour#2769.

Signed-off-by: James Peach <[email protected]>
  • Loading branch information
@jpeach
jpeach authored Aug 12, 2020
1 parent 215fb5b commit 5c7ceb2
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 76 deletions.
6 changes: 3 additions & 3 deletions internal/contour/visitor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestVisitClusters(t *testing.T) {
Upstream: &dag.Service{
Name: "example",
Namespace: "default",
ServicePort: &v1.ServicePort{
ServicePort: v1.ServicePort{
Protocol: "TCP",
Port: 443,
TargetPort: intstr.FromInt(8443),
Expand Down Expand Up @@ -86,7 +86,7 @@ func TestVisitListeners(t *testing.T) {
Upstream: &dag.Service{
Name: "example",
Namespace: "default",
ServicePort: &v1.ServicePort{
ServicePort: v1.ServicePort{
Protocol: "TCP",
Port: 443,
TargetPort: intstr.FromInt(8443),
Expand Down Expand Up @@ -167,7 +167,7 @@ func TestVisitSecrets(t *testing.T) {
Upstream: &dag.Service{
Name: "example",
Namespace: "default",
ServicePort: &v1.ServicePort{
ServicePort: v1.ServicePort{
Protocol: "TCP",
Port: 443,
TargetPort: intstr.FromInt(8443),
Expand Down
6 changes: 3 additions & 3 deletions internal/dag/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (b *Builder) lookupService(m types.NamespacedName, port intstr.IntOrString)
return nil, fmt.Errorf("service %q not found", m)
}
for i := range svc.Spec.Ports {
p := &svc.Spec.Ports[i]
p := svc.Spec.Ports[i]
switch {
case int(p.Port) == port.IntValue():
return b.addService(svc, p), nil
Expand All @@ -119,7 +119,7 @@ func (b *Builder) lookupService(m types.NamespacedName, port intstr.IntOrString)
return nil, fmt.Errorf("port %q on service %q not matched", port.String(), m)
}

func (b *Builder) addService(svc *v1.Service, port *v1.ServicePort) *Service {
func (b *Builder) addService(svc *v1.Service, port v1.ServicePort) *Service {
s := &Service{
Name: svc.Name,
Namespace: svc.Namespace,
Expand All @@ -136,7 +136,7 @@ func (b *Builder) addService(svc *v1.Service, port *v1.ServicePort) *Service {
return s
}

func upstreamProtocol(svc *v1.Service, port *v1.ServicePort) string {
func upstreamProtocol(svc *v1.Service, port v1.ServicePort) string {
up := annotation.ParseUpstreamProtocols(svc.Annotations)
protocol := up[port.Name]
if protocol == "" {
Expand Down
70 changes: 35 additions & 35 deletions internal/dag/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4066,7 +4066,7 @@ func TestDAGInsert(t *testing.T) {
prefixroute("/", &Service{
Name: s3d.Name,
Namespace: s3d.Namespace,
ServicePort: &s3d.Spec.Ports[0],
ServicePort: s3d.Spec.Ports[0],
Protocol: "h2c",
}),
),
Expand All @@ -4086,7 +4086,7 @@ func TestDAGInsert(t *testing.T) {
prefixroute("/", &Service{
Name: s3e.Name,
Namespace: s3e.Namespace,
ServicePort: &s3e.Spec.Ports[0],
ServicePort: s3e.Spec.Ports[0],
Protocol: "h2",
}),
),
Expand All @@ -4106,7 +4106,7 @@ func TestDAGInsert(t *testing.T) {
prefixroute("/", &Service{
Name: s3f.Name,
Namespace: s3f.Namespace,
ServicePort: &s3f.Spec.Ports[0],
ServicePort: s3f.Spec.Ports[0],
Protocol: "tls",
}),
),
Expand All @@ -4127,7 +4127,7 @@ func TestDAGInsert(t *testing.T) {
prefixroute("/", &Service{
Name: s3a.Name,
Namespace: s3a.Namespace,
ServicePort: &s3a.Spec.Ports[0],
ServicePort: s3a.Spec.Ports[0],
Protocol: "h2c",
}),
),
Expand All @@ -4147,7 +4147,7 @@ func TestDAGInsert(t *testing.T) {
prefixroute("/", &Service{
Name: s3b.Name,
Namespace: s3b.Namespace,
ServicePort: &s3b.Spec.Ports[0],
ServicePort: s3b.Spec.Ports[0],
Protocol: "h2",
}),
),
Expand All @@ -4167,7 +4167,7 @@ func TestDAGInsert(t *testing.T) {
prefixroute("/", &Service{
Name: s3c.Name,
Namespace: s3c.Namespace,
ServicePort: &s3c.Spec.Ports[0],
ServicePort: s3c.Spec.Ports[0],
Protocol: "tls",
}),
),
Expand All @@ -4188,7 +4188,7 @@ func TestDAGInsert(t *testing.T) {
prefixroute("/", &Service{
Name: s1b.Name,
Namespace: s1b.Namespace,
ServicePort: &s1b.Spec.Ports[0],
ServicePort: s1b.Spec.Ports[0],
MaxConnections: 9000,
MaxPendingRequests: 4096,
MaxRequests: 404,
Expand All @@ -4212,15 +4212,15 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
Weight: 90,
}),
routeCluster("/b", &Cluster{
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
Weight: 60,
}),
Expand All @@ -4243,14 +4243,14 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
Weight: 90,
}, &Cluster{
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
Weight: 60,
},
Expand Down Expand Up @@ -4526,7 +4526,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1a.Name,
Namespace: s1a.Namespace,
ServicePort: &s1a.Spec.Ports[0],
ServicePort: s1a.Spec.Ports[0],
Protocol: "tls",
},
Protocol: "tls",
Expand Down Expand Up @@ -4735,7 +4735,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
},
),
Expand All @@ -4744,7 +4744,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s4.Name,
Namespace: s4.Namespace,
ServicePort: &s4.Spec.Ports[0],
ServicePort: s4.Spec.Ports[0],
},
},
),
Expand All @@ -4767,7 +4767,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
},
),
Expand All @@ -4777,7 +4777,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s4.Name,
Namespace: s4.Namespace,
ServicePort: &s4.Spec.Ports[0],
ServicePort: s4.Spec.Ports[0],
},
}},
},
Expand All @@ -4800,7 +4800,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
},
),
Expand All @@ -4810,7 +4810,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s4.Name,
Namespace: s4.Namespace,
ServicePort: &s4.Spec.Ports[0],
ServicePort: s4.Spec.Ports[0],
},
}},
},
Expand All @@ -4819,7 +4819,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s4.Name,
Namespace: s4.Namespace,
ServicePort: &s4.Spec.Ports[0],
ServicePort: s4.Spec.Ports[0],
},
},
),
Expand All @@ -4829,7 +4829,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s11.Name,
Namespace: s11.Namespace,
ServicePort: &s11.Spec.Ports[0],
ServicePort: s11.Spec.Ports[0],
},
}},
},
Expand All @@ -4852,7 +4852,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
},
),
Expand All @@ -4861,7 +4861,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s2.Name,
Namespace: s2.Namespace,
ServicePort: &s2.Spec.Ports[0],
ServicePort: s2.Spec.Ports[0],
},
},
),
Expand All @@ -4884,7 +4884,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
},
),
Expand All @@ -4893,7 +4893,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s2.Name,
Namespace: s2.Namespace,
ServicePort: &s2.Spec.Ports[0],
ServicePort: s2.Spec.Ports[0],
},
},
),
Expand All @@ -4916,7 +4916,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
},
),
Expand All @@ -4925,7 +4925,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s2.Name,
Namespace: s2.Namespace,
ServicePort: &s2.Spec.Ports[0],
ServicePort: s2.Spec.Ports[0],
},
},
),
Expand All @@ -4948,7 +4948,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
},
),
Expand All @@ -4957,7 +4957,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s2.Name,
Namespace: s2.Namespace,
ServicePort: &s2.Spec.Ports[0],
ServicePort: s2.Spec.Ports[0],
},
},
),
Expand All @@ -4980,7 +4980,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s1.Name,
Namespace: s1.Namespace,
ServicePort: &s1.Spec.Ports[0],
ServicePort: s1.Spec.Ports[0],
},
},
),
Expand All @@ -4989,7 +4989,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s2.Name,
Namespace: s2.Namespace,
ServicePort: &s2.Spec.Ports[0],
ServicePort: s2.Spec.Ports[0],
},
},
),
Expand Down Expand Up @@ -5083,7 +5083,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s10.Name,
Namespace: s10.Namespace,
ServicePort: &s10.Spec.Ports[1],
ServicePort: s10.Spec.Ports[1],
},
},
),
Expand Down Expand Up @@ -5339,7 +5339,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s14.Name,
Namespace: s14.Namespace,
ServicePort: &s14.Spec.Ports[0],
ServicePort: s14.Spec.Ports[0],
ExternalName: "externalservice.io",
},
SNI: "externalservice.io",
Expand Down Expand Up @@ -5387,7 +5387,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s14.Name,
Namespace: s14.Namespace,
ServicePort: &s14.Spec.Ports[0],
ServicePort: s14.Spec.Ports[0],
ExternalName: "externalservice.io",
},
SNI: "bar.com",
Expand Down Expand Up @@ -5438,7 +5438,7 @@ func TestDAGInsert(t *testing.T) {
Upstream: &Service{
Name: s14.Name,
Namespace: s14.Namespace,
ServicePort: &s14.Spec.Ports[0],
ServicePort: s14.Spec.Ports[0],
ExternalName: "externalservice.io",
},
RequestHeadersPolicy: &HeadersPolicy{
Expand Down Expand Up @@ -6731,7 +6731,7 @@ func service(s *v1.Service) *Service {
return &Service{
Name: s.Name,
Namespace: s.Namespace,
ServicePort: &s.Spec.Ports[0],
ServicePort: s.Spec.Ports[0],
}
}

Expand Down
Loading

0 comments on commit 5c7ceb2

Please sign in to comment.