Skip to content
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.

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

Refactor setting the "resource" label into endpointsForHostname #3762

Merged
merged 2 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion source/ambassador_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ func (sc *ambassadorHostSource) endpointsFromHost(ctx context.Context, host *amb
providerSpecific := endpoint.ProviderSpecific{}
setIdentifier := ""

resource := fmt.Sprintf("host/%s/%s", host.Namespace, host.Name)

annotations := host.Annotations
ttl, err := getTTLFromAnnotations(annotations)
if err != nil {
Expand All @@ -176,7 +178,7 @@ func (sc *ambassadorHostSource) endpointsFromHost(ctx context.Context, host *amb
if host.Spec != nil {
hostname := host.Spec.Hostname
if hostname != "" {
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
}

Expand Down
21 changes: 9 additions & 12 deletions source/contour_httpproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ func (sc *httpProxySource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint,
}

log.Debugf("Endpoints generated from HTTPProxy: %s/%s: %v", hp.Namespace, hp.Name, hpEndpoints)
sc.setResourceLabel(hp, hpEndpoints)
endpoints = append(endpoints, hpEndpoints...)
}

Expand Down Expand Up @@ -206,9 +205,11 @@ func (sc *httpProxySource) endpointsFromTemplate(httpProxy *projectcontour.HTTPP

providerSpecific, setIdentifier := getProviderSpecificAnnotations(httpProxy.Annotations)

resource := fmt.Sprintf("HTTPProxy/%s/%s", httpProxy.Namespace, httpProxy.Name)

var endpoints []*endpoint.Endpoint
for _, hostname := range hostnames {
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
return endpoints, nil
}
Expand Down Expand Up @@ -244,21 +245,13 @@ func (sc *httpProxySource) filterByAnnotations(httpProxies []*projectcontour.HTT
return filteredList, nil
}

func (sc *httpProxySource) setResourceLabel(httpProxy *projectcontour.HTTPProxy, endpoints []*endpoint.Endpoint) {
for _, ep := range endpoints {
ep.Labels[endpoint.ResourceLabelKey] = fmt.Sprintf("HTTPProxy/%s/%s", httpProxy.Namespace, httpProxy.Name)
}
}

// endpointsFromHTTPProxyConfig extracts the endpoints from a Contour HTTPProxy object
func (sc *httpProxySource) endpointsFromHTTPProxy(httpProxy *projectcontour.HTTPProxy) ([]*endpoint.Endpoint, error) {
if httpProxy.Status.CurrentStatus != "valid" {
log.Warn(errors.Errorf("cannot generate endpoints for HTTPProxy with status %s", httpProxy.Status.CurrentStatus))
return nil, nil
}

var endpoints []*endpoint.Endpoint

ttl, err := getTTLFromAnnotations(httpProxy.Annotations)
if err != nil {
log.Warn(err)
Expand All @@ -279,17 +272,21 @@ func (sc *httpProxySource) endpointsFromHTTPProxy(httpProxy *projectcontour.HTTP

providerSpecific, setIdentifier := getProviderSpecificAnnotations(httpProxy.Annotations)

resource := fmt.Sprintf("HTTPProxy/%s/%s", httpProxy.Namespace, httpProxy.Name)

var endpoints []*endpoint.Endpoint

if virtualHost := httpProxy.Spec.VirtualHost; virtualHost != nil {
if fqdn := virtualHost.Fqdn; fqdn != "" {
endpoints = append(endpoints, endpointsForHostname(fqdn, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(fqdn, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
}

// Skip endpoints if we do not want entries from annotations
if !sc.ignoreHostnameAnnotation {
hostnameList := getHostnamesFromAnnotations(httpProxy.Annotations)
for _, hostname := range hostnameList {
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
}

Expand Down
8 changes: 2 additions & 6 deletions source/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,18 +228,14 @@ func (src *gatewayRouteSource) Endpoints(ctx context.Context) ([]*endpoint.Endpo
}

// Create endpoints from hostnames and targets.
resourceKey := fmt.Sprintf("%s/%s/%s", kind, meta.Namespace, meta.Name)
resource := fmt.Sprintf("%s/%s/%s", kind, meta.Namespace, meta.Name)
providerSpecific, setIdentifier := getProviderSpecificAnnotations(annots)
ttl, err := getTTLFromAnnotations(annots)
if err != nil {
log.Warn(err)
}
for host, targets := range hostTargets {
eps := endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier)
for _, ep := range eps {
ep.Labels[endpoint.ResourceLabelKey] = resourceKey
}
endpoints = append(endpoints, eps...)
endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
log.Debugf("Endpoints generated from %s %s/%s: %v", src.rtKind, meta.Namespace, meta.Name, endpoints)
}
Expand Down
2 changes: 1 addition & 1 deletion source/gloo_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func (gs *glooSource) generateEndpointsFromProxy(ctx context.Context, proxy *pro
}
providerSpecific, setIdentifier := getProviderSpecificAnnotations(annotations)
for _, domain := range virtualHost.Domains {
endpoints = append(endpoints, endpointsForHostname(strings.TrimSuffix(domain, "."), targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(strings.TrimSuffix(domain, "."), targets, ttl, providerSpecific, setIdentifier, "")...)
}
}
Comment on lines 160 to 164
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird to have only Gloo Proxy without resource Label. It's because they only use annotations ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably should list the VirtualHost. But that is a pre-existing issue.

}
Expand Down
19 changes: 8 additions & 11 deletions source/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ func (sc *ingressSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, e
}

log.Debugf("Endpoints generated from ingress: %s/%s: %v", ing.Namespace, ing.Name, ingEndpoints)
sc.setResourceLabel(ing, ingEndpoints)
sc.setDualstackLabel(ing, ingEndpoints)
endpoints = append(endpoints, ingEndpoints...)
}
Expand Down Expand Up @@ -205,9 +204,11 @@ func (sc *ingressSource) endpointsFromTemplate(ing *networkv1.Ingress) ([]*endpo

providerSpecific, setIdentifier := getProviderSpecificAnnotations(ing.Annotations)

resource := fmt.Sprintf("ingress/%s/%s", ing.Namespace, ing.Name)

var endpoints []*endpoint.Endpoint
for _, hostname := range hostnames {
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
return endpoints, nil
}
Expand Down Expand Up @@ -280,12 +281,6 @@ func (sc *ingressSource) filterByIngressClass(ingresses []*networkv1.Ingress) ([
return filteredList, nil
}

func (sc *ingressSource) setResourceLabel(ingress *networkv1.Ingress, endpoints []*endpoint.Endpoint) {
for _, ep := range endpoints {
ep.Labels[endpoint.ResourceLabelKey] = fmt.Sprintf("ingress/%s/%s", ingress.Namespace, ingress.Name)
}
}

func (sc *ingressSource) setDualstackLabel(ingress *networkv1.Ingress, endpoints []*endpoint.Endpoint) {
val, ok := ingress.Annotations[ALBDualstackAnnotationKey]
if ok && val == ALBDualstackAnnotationValue {
Expand All @@ -311,6 +306,8 @@ func endpointsFromIngress(ing *networkv1.Ingress, ignoreHostnameAnnotation bool,

providerSpecific, setIdentifier := getProviderSpecificAnnotations(ing.Annotations)

resource := fmt.Sprintf("ingress/%s/%s", ing.Namespace, ing.Name)

// Gather endpoints defined on hosts sections of the ingress
var definedHostsEndpoints []*endpoint.Endpoint
// Skip endpoints if we do not want entries from Rules section
Expand All @@ -319,7 +316,7 @@ func endpointsFromIngress(ing *networkv1.Ingress, ignoreHostnameAnnotation bool,
if rule.Host == "" {
continue
}
definedHostsEndpoints = append(definedHostsEndpoints, endpointsForHostname(rule.Host, targets, ttl, providerSpecific, setIdentifier)...)
definedHostsEndpoints = append(definedHostsEndpoints, endpointsForHostname(rule.Host, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
}

Expand All @@ -330,7 +327,7 @@ func endpointsFromIngress(ing *networkv1.Ingress, ignoreHostnameAnnotation bool,
if host == "" {
continue
}
definedHostsEndpoints = append(definedHostsEndpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier)...)
definedHostsEndpoints = append(definedHostsEndpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
}
}
Expand All @@ -339,7 +336,7 @@ func endpointsFromIngress(ing *networkv1.Ingress, ignoreHostnameAnnotation bool,
var annotationEndpoints []*endpoint.Endpoint
if !ignoreHostnameAnnotation {
for _, hostname := range getHostnamesFromAnnotations(ing.Annotations) {
annotationEndpoints = append(annotationEndpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
annotationEndpoints = append(annotationEndpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
}

Expand Down
11 changes: 3 additions & 8 deletions source/istio_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ func (sc *gatewaySource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, e
}

log.Debugf("Endpoints generated from gateway: %s/%s: %v", gateway.Namespace, gateway.Name, gwEndpoints)
sc.setResourceLabel(gateway, gwEndpoints)
endpoints = append(endpoints, gwEndpoints...)
}

Expand Down Expand Up @@ -230,12 +229,6 @@ func (sc *gatewaySource) filterByAnnotations(gateways []*networkingv1alpha3.Gate
return filteredList, nil
}

func (sc *gatewaySource) setResourceLabel(gateway *networkingv1alpha3.Gateway, endpoints []*endpoint.Endpoint) {
for _, ep := range endpoints {
ep.Labels[endpoint.ResourceLabelKey] = fmt.Sprintf("gateway/%s/%s", gateway.Namespace, gateway.Name)
}
}

func parseIngress(ingress string) (namespace, name string, err error) {
parts := strings.Split(ingress, "/")
if len(parts) == 2 {
Expand Down Expand Up @@ -329,8 +322,10 @@ func (sc *gatewaySource) endpointsFromGateway(ctx context.Context, hostnames []s

providerSpecific, setIdentifier := getProviderSpecificAnnotations(annotations)

resource := fmt.Sprintf("gateway/%s/%s", gateway.Namespace, gateway.Name)

for _, host := range hostnames {
endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...)
}

return endpoints, nil
Expand Down
17 changes: 7 additions & 10 deletions source/istio_virtualservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ func (sc *virtualServiceSource) Endpoints(ctx context.Context) ([]*endpoint.Endp
}

log.Debugf("Endpoints generated from VirtualService: %s/%s: %v", virtualService.Namespace, virtualService.Name, gwEndpoints)
sc.setResourceLabel(virtualService, gwEndpoints)
endpoints = append(endpoints, gwEndpoints...)
}

Expand Down Expand Up @@ -230,13 +229,15 @@ func (sc *virtualServiceSource) endpointsFromTemplate(ctx context.Context, virtu

providerSpecific, setIdentifier := getProviderSpecificAnnotations(virtualService.Annotations)

resource := fmt.Sprintf("virtualservice/%s/%s", virtualService.Namespace, virtualService.Name)

var endpoints []*endpoint.Endpoint
for _, hostname := range hostnames {
targets, err := sc.targetsFromVirtualService(ctx, virtualService, hostname)
if err != nil {
return endpoints, err
}
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
return endpoints, nil
}
Expand Down Expand Up @@ -272,12 +273,6 @@ func (sc *virtualServiceSource) filterByAnnotations(virtualservices []*networkin
return filteredList, nil
}

func (sc *virtualServiceSource) setResourceLabel(virtualservice *networkingv1alpha3.VirtualService, endpoints []*endpoint.Endpoint) {
for _, ep := range endpoints {
ep.Labels[endpoint.ResourceLabelKey] = fmt.Sprintf("virtualservice/%s/%s", virtualservice.Namespace, virtualservice.Name)
}
}

// append a target to the list of targets unless it's already in the list
func appendUnique(targets []string, target string) []string {
for _, element := range targets {
Expand Down Expand Up @@ -327,6 +322,8 @@ func (sc *virtualServiceSource) endpointsFromVirtualService(ctx context.Context,

providerSpecific, setIdentifier := getProviderSpecificAnnotations(virtualservice.Annotations)

resource := fmt.Sprintf("virtualservice/%s/%s", virtualservice.Namespace, virtualservice.Name)

for _, host := range virtualservice.Spec.Hosts {
if host == "" || host == "*" {
continue
Expand All @@ -348,7 +345,7 @@ func (sc *virtualServiceSource) endpointsFromVirtualService(ctx context.Context,
}
}

endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...)
}

// Skip endpoints if we do not want entries from annotations
Expand All @@ -362,7 +359,7 @@ func (sc *virtualServiceSource) endpointsFromVirtualService(ctx context.Context,
return endpoints, err
}
}
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
}

Expand Down
17 changes: 6 additions & 11 deletions source/kong_tcpingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ func (sc *kongTCPIngressSource) Endpoints(ctx context.Context) ([]*endpoint.Endp
}

log.Debugf("Endpoints generated from TCPIngress: %s: %v", fullname, ingressEndpoints)
sc.setResourceLabel(tcpIngress, ingressEndpoints)
sc.setDualstackLabel(tcpIngress, ingressEndpoints)
endpoints = append(endpoints, ingressEndpoints...)
}
Expand Down Expand Up @@ -189,12 +188,6 @@ func (sc *kongTCPIngressSource) filterByAnnotations(tcpIngresses []*TCPIngress)
return filteredList, nil
}

func (sc *kongTCPIngressSource) setResourceLabel(tcpIngress *TCPIngress, endpoints []*endpoint.Endpoint) {
for _, ep := range endpoints {
ep.Labels[endpoint.ResourceLabelKey] = fmt.Sprintf("tcpingress/%s/%s", tcpIngress.Namespace, tcpIngress.Name)
}
}

func (sc *kongTCPIngressSource) setDualstackLabel(tcpIngress *TCPIngress, endpoints []*endpoint.Endpoint) {
val, ok := tcpIngress.Annotations[ALBDualstackAnnotationKey]
if ok && val == ALBDualstackAnnotationValue {
Expand All @@ -209,22 +202,24 @@ func (sc *kongTCPIngressSource) setDualstackLabel(tcpIngress *TCPIngress, endpoi
func (sc *kongTCPIngressSource) endpointsFromTCPIngress(tcpIngress *TCPIngress, targets endpoint.Targets) ([]*endpoint.Endpoint, error) {
var endpoints []*endpoint.Endpoint

providerSpecific, setIdentifier := getProviderSpecificAnnotations(tcpIngress.Annotations)

ttl, err := getTTLFromAnnotations(tcpIngress.Annotations)
if err != nil {
return nil, err
}

providerSpecific, setIdentifier := getProviderSpecificAnnotations(tcpIngress.Annotations)

resource := fmt.Sprintf("tcpingress/%s/%s", tcpIngress.Namespace, tcpIngress.Name)

hostnameList := getHostnamesFromAnnotations(tcpIngress.Annotations)
for _, hostname := range hostnameList {
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}

if tcpIngress.Spec.Rules != nil {
for _, rule := range tcpIngress.Spec.Rules {
if rule.Host != "" {
endpoints = append(endpoints, endpointsForHostname(rule.Host, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(rule.Host, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion source/multisource.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (ms *multiSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, err
}
if len(ms.defaultTargets) > 0 {
for i := range endpoints {
eps := endpointsForHostname(endpoints[i].DNSName, ms.defaultTargets, endpoints[i].RecordTTL, endpoints[i].ProviderSpecific, endpoints[i].SetIdentifier)
eps := endpointsForHostname(endpoints[i].DNSName, ms.defaultTargets, endpoints[i].RecordTTL, endpoints[i].ProviderSpecific, endpoints[i].SetIdentifier, "")
for _, ep := range eps {
ep.Labels = endpoints[i].Labels
}
Comment on lines 39 to 45
Copy link
Contributor

@mloiseleur mloiseleur Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With two sources, the second one may erase resource labels of the first, no ? 🤔

Copy link
Contributor Author

@johngmyers johngmyers Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would that happen? The code, both before and after, is copying all the labels for each endpoint it is copying (and applying defaultTargets. Each sub-source's endpoints will get their respective resource labels.

Expand Down
17 changes: 7 additions & 10 deletions source/openshift_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ func (ors *ocpRouteSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint,
}

log.Debugf("Endpoints generated from OpenShift Route: %s/%s: %v", ocpRoute.Namespace, ocpRoute.Name, orEndpoints)
ors.setResourceLabel(ocpRoute, orEndpoints)
endpoints = append(endpoints, orEndpoints...)
}

Expand Down Expand Up @@ -188,9 +187,11 @@ func (ors *ocpRouteSource) endpointsFromTemplate(ocpRoute *routev1.Route) ([]*en

providerSpecific, setIdentifier := getProviderSpecificAnnotations(ocpRoute.Annotations)

resource := fmt.Sprintf("route/%s/%s", ocpRoute.Namespace, ocpRoute.Name)

var endpoints []*endpoint.Endpoint
for _, hostname := range hostnames {
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
return endpoints, nil
}
Expand Down Expand Up @@ -225,12 +226,6 @@ func (ors *ocpRouteSource) filterByAnnotations(ocpRoutes []*routev1.Route) ([]*r
return filteredList, nil
}

func (ors *ocpRouteSource) setResourceLabel(ocpRoute *routev1.Route, endpoints []*endpoint.Endpoint) {
for _, ep := range endpoints {
ep.Labels[endpoint.ResourceLabelKey] = fmt.Sprintf("route/%s/%s", ocpRoute.Namespace, ocpRoute.Name)
}
}

// endpointsFromOcpRoute extracts the endpoints from a OpenShift Route object
func (ors *ocpRouteSource) endpointsFromOcpRoute(ocpRoute *routev1.Route, ignoreHostnameAnnotation bool) []*endpoint.Endpoint {
var endpoints []*endpoint.Endpoint
Expand All @@ -249,15 +244,17 @@ func (ors *ocpRouteSource) endpointsFromOcpRoute(ocpRoute *routev1.Route, ignore

providerSpecific, setIdentifier := getProviderSpecificAnnotations(ocpRoute.Annotations)

resource := fmt.Sprintf("route/%s/%s", ocpRoute.Namespace, ocpRoute.Name)

if host != "" {
endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(host, targets, ttl, providerSpecific, setIdentifier, resource)...)
}

// Skip endpoints if we do not want entries from annotations
if !ignoreHostnameAnnotation {
hostnameList := getHostnamesFromAnnotations(ocpRoute.Annotations)
for _, hostname := range hostnameList {
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier)...)
endpoints = append(endpoints, endpointsForHostname(hostname, targets, ttl, providerSpecific, setIdentifier, resource)...)
}
}
return endpoints
Expand Down
Loading
Loading