feat: adding support for creating targets from ExternalName #3571

msvticket · 2024-02-12T19:16:29Z

Issue

#3505

Description

I added support for ip target groups when service is of type ExternalName.

I have not updated any documentation, but I'm happy to do that in case you want the code change to begin with.

Checklist

Added tests that cover your change (if possible)
Added/modified documentation as required (such as the README.md, or the docs directory)
Manually tested
Made sure the title of the PR is a good description that can go into the release notes

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Backfilled missing tests for code in same general area 🎉
Refactored something and made the world a better place 🌟

k8s-ci-robot · 2024-02-12T19:16:37Z

Welcome @msvticket!

It looks like this is your first PR to kubernetes-sigs/aws-load-balancer-controller 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/aws-load-balancer-controller has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-02-12T19:16:38Z

Hi @msvticket. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

msvticket · 2024-05-08T08:25:32Z

Could you have a look at this @oliviassss ?

Leen15 · 2024-05-30T10:28:20Z

any update on this?

MateuszWkDev · 2024-07-11T14:31:23Z

I woul like to get update on this as this seems much needed functionality in order to support certain usecases

Reyalsorik · 2024-08-01T23:38:23Z

Any update? I would appreciate this functionality.

nousot-cloud-guy · 2024-08-29T14:00:02Z

Is there anything that's holding up this PR that someone could help finish? This would be really useful functionality to have.

msvticket · 2024-09-19T13:19:24Z

Is this something you can look at @shraddhabang?

felipegouveiae · 2024-10-26T02:02:17Z

@johngmyers @M00nF1sh please review and merge! I'm begging you!

ryantate13

I'm excited about this functionality getting built. I do think we need to be cognizant of dependents on this package and maintain a stable API surface for v2 however. Seems like some simple fix ups should get is back into that territory.

ryantate13 · 2025-02-26T09:22:15Z

pkg/backend/endpoint_resolver.go

@@ -27,12 +28,17 @@ var ErrNotFound = errors.New("backend not found")
 type EndpointResolver interface {
 	// ResolvePodEndpoints will resolve endpoints backed by pods directly.
 	// returns resolved podEndpoints and whether there are unready endpoints that can potentially turn ready in future reconciles.
-	ResolvePodEndpoints(ctx context.Context, svcKey types.NamespacedName, port intstr.IntOrString,
-		opts ...EndpointResolveOption) ([]PodEndpoint, bool, error)
+	ResolvePodEndpoints(ctx context.Context, svckey types.NamespacedName, svc *corev1.Service, port intstr.IntOrString, opts ...EndpointResolveOption) ([]IpEndpoint, bool, error)


There's no need for a breaking API change here. The variadic list of EndpointResolveOption argument should eliminate that necessity, can just create a new one to the effect of WithService(*corev1.Service). This package is currently at v2, merging this would require tagging a v3 and that creates more churn for adoption than necessary.

ryantate13 · 2025-02-26T09:23:10Z

pkg/backend/endpoint_types.go

-// An endpoint provided by pod directly.
-type PodEndpoint struct {
+// IpEndpoint is an endpoint for an ip address
+type IpEndpoint struct {


Name here should remain stable

ryantate13 · 2025-02-26T09:23:32Z

pkg/backend/endpoint_types.go

 	// Pod's IP.
 	IP string
 	// Pod's container port.
 	Port int32
 	// Pod that provides this endpoint.
-	Pod k8s.PodInfo
+	Pod *k8s.PodInfo


This needs to remain backwards compatible

msvticket · 2025-02-26T09:35:27Z

I'm excited about this functionality getting built. I do think we need to be cognizant of dependents on this package and maintain a stable API surface for v2 however. Seems like some simple fix ups should get is back into that territory.

I don't mind putting some more time in this PR if it could be merged. But since this PR is ignored by the maintainers I don't see any point in spending that time now.

msvticket · 2025-05-07T13:26:29Z

Hi @zac-nixon!

You seem to be a newish active maintainer of aws-load-balancer-controller. Can you please review this old PR? I've just rebased it on main, built it and tested it on a staging cluster.

deens-cyera · 2025-06-23T08:24:10Z

Hey, any update in the pr ?

chris-codaio · 2025-08-19T22:56:40Z

Another vote to get this reviewed & committed - just ran into this limitation this week!

zac-nixon · 2025-08-20T20:43:32Z

pkg/ingress/model_build_target_group.go

 	rawTargetType := string(t.defaultTargetType)
 	_ = t.annotationParser.ParseStringAnnotation(annotations.IngressSuffixTargetType, &rawTargetType, svcAndIngAnnotations)
 	if classCfg.IngClassParams != nil && classCfg.IngClassParams.Spec.TargetType != "" {
 		rawTargetType = string(classCfg.IngClassParams.Spec.TargetType)
 	}
+	if svc.Spec.Type == corev1.ServiceTypeExternalName && rawTargetType != string(elbv2model.TargetTypeIP) {
+		t.logger.Info("Target type will be ip since service is an ExternalName", "service", k8s.NamespacedName(svc))
+		rawTargetType = string(elbv2model.TargetTypeIP)


I'd recommend throwing an error rather then overwriting this value.

I think returning an error if something else than ip is set in an annotation makes sense, but still default to ip. What do you think about that?

I have now pushed a commit with that change

…nalName

k8s-ci-robot · 2025-08-21T12:51:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: msvticket
Once this PR has been reviewed and has the lgtm label, please ask for approval from zac-nixon. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

zac-nixon · 2025-08-26T01:25:49Z

Hi. I'd like to update everyone on the conversation we had in Slack.

Zac Nixon
  [1:55 PM](https://kubernetes.slack.com/archives/D09ARVB64TF/p1755723355218379)
I reviewed the PR, the code your wrote makes sense but I have a concern. How does the controller know that the DNS record has been updated, so that it can update the target group? Right now, you're relying the sync time (~10 hours) to do the DNS updates. It seems that we would need some background process that is querying dns periodically and enqueuing events when the record changes.


Mårten Svantesson
  [8:51 AM](https://kubernetes.slack.com/archives/D09ARVB64TF/p1755791499599059)
Well, my thought is that this is an initial implementation that I tried to keep as simple as possible. You are right in that it probably wont suffice for everybody, but I think it is better to base improvements on feedback rather than guesses.


Zac Nixon
  [7:53 AM](https://kubernetes.slack.com/archives/D09ARVB64TF/p1755874427108159)
Yeah, I see what you're saying. I really appreciate the PR, but generally we don't accept features that aren't complete. Not having a way to timely update the LoadBalancer when the DNS entries changes seems like a big problem. Can you explain how this feature helps you out?


Mårten Svantesson
  [7:09 AM](https://kubernetes.slack.com/archives/D09ARVB64TF/p1756130961627789)
I use it to implement this solution: https://aws.amazon.com/blogs/networking-and-content-delivery/hosting-internal-https-static-websites-with-alb-s3-and-privatelink/
By using an ExternalName for s3 I don’t need a separate ALB but can use the same that is used for accessing internal application running in EKS.
Amazon Web ServicesAmazon Web Services
[Hosting Internal HTTPS Static Websites with ALB, S3, and PrivateLink | Amazon Web Services](https://aws.amazon.com/blogs/networking-and-content-delivery/hosting-internal-https-static-websites-with-alb-s3-and-privatelink/)
Amazon Simple Storage Service (Amazon S3) is a powerful platform that enables you to do various tasks. One notable feature is the ability to create a bucket with an FQDN, point an alias record to the bucket website endpoint, and immediately get up-and-running with an HTTP static website. If you want to serve HTTPS traffic […]
Dec 30th, 2022
https://aws.amazon.com/blogs/networking-and-content-delivery/hosting-internal-https-static-websites-with-alb-s3-and-privatelink/



Zac Nixon
  [8:36 AM](https://kubernetes.slack.com/archives/D09ARVB64TF/p1756136188696239)
Thanks for the link. I suspect that this works for your case because AFAIK, Private Link uses a static IP address.
[8:37](https://kubernetes.slack.com/archives/D09ARVB64TF/p1756136238044719)
Generally, we would like to supply more general solutions, this one has a pretty sharp edge where Target Group changes aren't propagated very fast.

TL;DR - There is some more work to be done here to accept this change. I don't really have the time until ~October to put more stuff on my plate.

k8s-ci-robot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Feb 12, 2024

k8s-ci-robot requested review from johngmyers and M00nF1sh February 12, 2024 19:16

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 12, 2024

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 12, 2024

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 12, 2024

msvticket force-pushed the externalname branch from 8c92a38 to 7b34837 Compare February 12, 2024 19:17

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Feb 12, 2024

msvticket force-pushed the externalname branch from 7b34837 to 21588bb Compare February 20, 2024 14:50

msvticket changed the title ~~Adding support for creating targets from ExternalName~~ feat: adding support for creating targets from ExternalName Mar 1, 2024

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 16, 2024

msvticket force-pushed the externalname branch 2 times, most recently from db436ab to e265988 Compare March 16, 2024 11:49

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 16, 2024

msvticket force-pushed the externalname branch from e265988 to f673254 Compare May 27, 2024 12:47

Varun-Mehrotra mentioned this pull request May 28, 2024

namespacedNameError on Proxy when attempting to submit a cURL call to it kedacore/http-add-on#1041

Closed

msvticket mentioned this pull request May 29, 2024

Lack of ExternalName Service support #3505

Open

wozniakjan mentioned this pull request Jun 4, 2024

Multiple deployments per httpso kedacore/http-add-on#1053

Closed

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 12, 2024

msvticket force-pushed the externalname branch 2 times, most recently from f553585 to bffcdc0 Compare December 11, 2024 10:43

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 11, 2024

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 28, 2025

ryantate13 suggested changes Feb 26, 2025

View reviewed changes

msvticket force-pushed the externalname branch from bffcdc0 to 976ad25 Compare May 6, 2025 15:55

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2025

msvticket force-pushed the externalname branch 2 times, most recently from 0fb4be9 to 7f44c19 Compare May 7, 2025 12:07

msvticket marked this pull request as draft May 7, 2025 13:25

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 7, 2025

msvticket marked this pull request as ready for review May 7, 2025 13:25

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 7, 2025

k8s-ci-robot requested review from oliviassss and shraddhabang May 7, 2025 13:25

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2025

feat: support externalname services

623df90

msvticket force-pushed the externalname branch from 7f44c19 to 623df90 Compare August 18, 2025 09:35

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2025

zac-nixon reviewed Aug 20, 2025

View reviewed changes

zac-nixon assigned zac-nixon and unassigned zac-nixon Aug 20, 2025

fix: error if target tpe is set explicitly to other than ip for Exter…

ab1415f

…nalName

feat: adding support for creating targets from ExternalName #3571

Are you sure you want to change the base?

feat: adding support for creating targets from ExternalName #3571

Conversation

msvticket commented Feb 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description

Checklist

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Uh oh!

k8s-ci-robot commented Feb 12, 2024

Uh oh!

k8s-ci-robot commented Feb 12, 2024

Uh oh!

msvticket commented May 8, 2024

Uh oh!

Leen15 commented May 30, 2024

Uh oh!

MateuszWkDev commented Jul 11, 2024

Uh oh!

Reyalsorik commented Aug 1, 2024

Uh oh!

nousot-cloud-guy commented Aug 29, 2024

Uh oh!

msvticket commented Sep 19, 2024

Uh oh!

felipegouveiae commented Oct 26, 2024

Uh oh!

ryantate13 left a comment

Choose a reason for hiding this comment

Uh oh!

ryantate13 Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

ryantate13 Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

ryantate13 Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

msvticket commented Feb 26, 2025

Uh oh!

msvticket commented May 7, 2025

Uh oh!

deens-cyera commented Jun 23, 2025

Uh oh!

chris-codaio commented Aug 19, 2025

Uh oh!

zac-nixon Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

msvticket Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msvticket Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Aug 21, 2025

Uh oh!

zac-nixon commented Aug 26, 2025

Uh oh!

Uh oh!

msvticket commented Feb 12, 2024 •

edited

Loading

msvticket Aug 21, 2025 •

edited

Loading