chore(elbv2): add ListenerRuleHostHeaders #3473
Conversation
efekarakus
added
the
do-not-merge
| // ListenerRuleHostHeaders returns all the host headers for a listener rule. | ||
| func (e *ELBV2) ListenerRuleHostHeaders(ruleARN string) ([]string, error) { |
There was a problem hiding this comment.
Are you planning to have also a path-pattern method? so that we can concatenate the two and give an output to the clients?
There was a problem hiding this comment.
Hmm I was planning to continue using RulePath 🤔
| for _, value := range condition.Values { | ||
| hostHeaderSet[aws.StringValue(value)] = true | ||
| } |
There was a problem hiding this comment.
Wut, what's the difference between condition.Values and condition.HostHeaderConfig.Values?
If I look at our CFN templates, we only have the latter.
There was a problem hiding this comment.
Yeah but the results always have both two. I check values just to be safe...
There was a problem hiding this comment.
OK I just looked this up lol and these field names are so confusing 🤦
condition.Values allows you to specify only a single host name although the type is an array.
condition.HostHeaderConfig allows you to specify multiple host names.
So yeah, we should check both 👍 , maybe drop a comment with an explanation?
| for _, value := range condition.Values { | |
| hostHeaderSet[aws.StringValue(value)] = true | |
| } | |
| // Values is a legacy field that allowed specifying only a single host name. | |
| // The alternative is to use HostHeaderConfig for multiple values. | |
| // Only one of these fields should be set, but we collect from both to be safe. | |
| for _, value := range condition.Values { | |
| hostHeaderSet[aws.StringValue(value)] = true | |
| } |
| for _, value := range condition.Values { | ||
| hostHeaderSet[aws.StringValue(value)] = true | ||
| } |
There was a problem hiding this comment.
OK I just looked this up lol and these field names are so confusing 🤦
condition.Values allows you to specify only a single host name although the type is an array.
condition.HostHeaderConfig allows you to specify multiple host names.
So yeah, we should check both 👍 , maybe drop a comment with an explanation?
| for _, value := range condition.Values { | |
| hostHeaderSet[aws.StringValue(value)] = true | |
| } | |
| // Values is a legacy field that allowed specifying only a single host name. | |
| // The alternative is to use HostHeaderConfig for multiple values. | |
| // Only one of these fields should be set, but we collect from both to be safe. | |
| for _, value := range condition.Values { | |
| hostHeaderSet[aws.StringValue(value)] = true | |
| } |
internal/pkg/aws/elbv2/elbv2.go
Outdated
| hostHeaderSet[aws.StringValue(value)] = true | ||
| } | ||
| if condition.HostHeaderConfig == nil { | ||
| // Ideally this should never happen. |
There was a problem hiding this comment.
nit:
| // Ideally this should never happen. |
iamhopaul123
removed
the
do-not-merge
part of #2694
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.