Add TG protocol into TGB

[zac-nixon]

Description

Adds TG protocol in TGB for an upcoming project. Also refactors the webhook logic to only look up TG once per invocation, previously we would re-fetch the TG from ELB for every validation.

Scenarios tested:

Deploy new LBC, don't update TGB CRD.

• E2E tests still work.
• TGB operations still work.

Deploy old LBC, update TGB CRD.

• E2E tests still work
• TGB operations still work (protocol is just nil)

Deploy new LBC, update TGB CRD.

• E2E tests work.
• Tested without inserting protocol, it gets auto populated.
• Ingress / Service / Gateway API all auto populate protocol.
• Manually created TGB rejects unknown protocol types.
• Manually created TGB rejects protocol types when it doesn't matched the protocol stored within ELB.

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]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zac-nixon

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [zac-nixon]

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

[shuqz]

let's make this consistent: sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2/k8s i saw it was used as elbv2model, k8s2 and k8s

[zac-nixon]

sounds good. i was getting confused about the various names assigned. :)

[shuqz]

looks like for all getXXXFromAWS functions, we are doing

obj := tgCache() targetGroup := obj.tg err := obj.error if err != nil {	return "", err	} 

then maybe instead of pass in tgCache func() tgCacheObject, can we check error right after

targetGroupCache := sync.OnceValue(func() tgCacheObject {	targetGroup, err := getTargetGroupFromAWS(ctx, m.elbv2Client, tgb)	return tgCacheObject{	tg: targetGroup,	error: err,	}	}) 

and then pass targerGroup: tgCache().tg

[zac-nixon]

the idea was that we only want to call at most one time. in the current setup we only call getTargetGroupFromAWS when we need to, which means we need to have each invocation be prepared to handle an error. it sounds like your suggestion would be to just call getTargetGroupFromAWS and pass the value around?

lmk if i misunderstood.

[shuqz]

for functions getVpcIDFromAWS, getTargetGroupIPAddressTypeFromAWS, obtainSDKTargetGroupProtocolFromAWS etc, this part of code is in all of them

obj := tgCache() targetGroup := obj.tg err := obj.error if err != nil {	return "", err	} 

because we passed targetGroupCache in m.defaultingIPAddressType(tgb, targetGroupCache), i am thinking if we can check cache error before calling all those m.defaultXXXX functions and just pass m.defaultingIPAddressType(tgb, targetGroupCache.tg)
in this way, we only need to check err once and remove duplicated codes, let me know if this applies

[zac-nixon]

ah ok. i did understand what you were saying. I don't think we can do that, if we want to maintain the minimum amount of calls.

We only want to fetch the Target Group if we need to, so in this case we need each function to handle the error. For example, we might not need to fetch the TG prior to validating the TG VPC, if that call fails we need to have error handling.

[shuqz]

oh i see your point now, thanks

[shuqz]

/lgtm

[shuqz]

/approved

[shraddhabang]

/lgtm
/approved

[zac-nixon]

/retest