community.aws icon indicating copy to clipboard operation
community.aws copied to clipboard

elb_target_group: add support for protocol_version parameter

Open markuman opened this issue 1 year ago • 5 comments

Summary

elb_target_group cannot set the protocol_version parameter of a target group.

  • https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/elbv2.html#ElasticLoadBalancingv2.Client.create_target_group

ProtocolVersion (string) -- [HTTP/HTTPS protocol] The protocol version. Specify GRPC to send requests to targets using gRPC. Specify HTTP2 to send requests to targets using HTTP/2. The default is HTTP1 , which sends requests to targets using HTTP/1.1.

Issue Type

Feature Idea

Component Name

elb_target_group

Additional Information

the elb_target_group_info module does read the protocol_version parameter already

                "protocol": "HTTP",
                "protocol_version": "GRPC",

Code of Conduct

  • [X] I agree to follow the Ansible Code of Conduct

markuman avatar Aug 25 '22 05:08 markuman

Files identified in the description:

  • [plugins/modules/elb_target_group.py](https://github.com/['ansible-collections/amazon.aws', 'ansible-collections/community.aws', 'ansible-collections/community.vmware']/blob/main/plugins/modules/elb_target_group.py)
  • [plugins/modules/elb_target_group_info.py](https://github.com/['ansible-collections/amazon.aws', 'ansible-collections/community.aws', 'ansible-collections/community.vmware']/blob/main/plugins/modules/elb_target_group_info.py)

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot avatar Aug 25 '22 05:08 ansibullbot

cc @jillr @s-hertel @tremble click here for bot help

ansibullbot avatar Aug 25 '22 05:08 ansibullbot

Hello,

I was trying to use the code in the linked PR by @sharvarikhedkar to create some GRPC target groups. Unfortunately, that fails because when the ProtocolVersion is set to GRPC the Matcher object for healthcheck response codes now accepts and returns GrpcCode properties, but the current code always expects HttpCode. See the boto docs. Thus causing KeyError's to be thrown.

I fixed this in a commit in my own branch based on @sharvarikhedkar 's work: https://github.com/rbobrowicz/community.aws/commit/e0f9b206707804d4293a7fd9d896807a81e91123 and now it appears I can create GRPC groups successfully.

I'm just not sure what the process for getting that merged is. Should I open my own PR against this repo? Should I open a PR agains't her branch? Should she just lift the commit into her branch herself?

Let me know what I can do to help.

rbobrowicz avatar Sep 26 '22 03:09 rbobrowicz

I've tested @rbobrowicz version successfully, and would very much appreciate this making its way into the next release. Thank you!

jontow avatar Sep 26 '22 21:09 jontow

Hi @rbobrowicz Thank you for catching this potential bug and contribution on the code. I've cherry-picked your commit and the patch with your commit ID has been added to the PR. Given that it's already being reviewed, if you'd like to I'm open to collaborating on the same PR to get it merged quicker :)

@jontow Thank you for testing the code change!

sharvarikhedkar avatar Sep 27 '22 04:09 sharvarikhedkar