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

Add support for GCP extensions #81

Merged
merged 1 commit into from
Sep 7, 2021
Merged

Conversation

igor-kupczynski
Copy link
Contributor

The support is modeled on azure.go under tlvparse. The GCP spec is similar but
encodes uint64. There's no subtype. Since zero value of uint64 is technically
a valid header value, I've opted for an extra boolean to indicate if it was
found (same as with Azure).

Given that we already have AWS, and Azure in tlvparse I thought it will be a good idea to add the GCP support as well.

We use it with GCP in our pre-prod env and all seems to be working well.

Let me know if you think it makes sense to have this with the upstream lib.

@coveralls
Copy link

coveralls commented Sep 6, 2021

Coverage Status

Coverage remained the same at 94.353% when pulling 9f25ec7 on igor-kupczynski:gcp-tlv-parser into 3aa7ea9 on pires:main.

The support is modeled on azure.go under tlvparse. The GCP spec is similar but
encodes uint64. There's no subtype. Since zero value of uint64 is technically
a valid header value, I've opted for an extra boolean to indicate if it was
found (same as with Azure).
@igor-kupczynski
Copy link
Contributor Author

I've noticed a naming inconsistency and force-pushed a fixed version. Hope that OK -- my thinking was that a simple rename doesn't add a lot to the history.

Copy link
Owner

@pires pires left a comment

Choose a reason for hiding this comment

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

The only thing I'd like to see here and maybe it's something for the future is to provide some structure, such as func names tied to PP2 type, eg PP2GCP.ExtractPSCConnectionID, or maybe have tlvparse/{aws,azure,gcp} packages so folks don't need to read the code to know which functions are available to them for a particular type.

That said, thank you very much for your contributions! 🙇🏻

@pires pires merged commit 10d5fbd into pires:main Sep 7, 2021
@igor-kupczynski
Copy link
Contributor Author

Nice, thanks for merging :) and thanks for maintaining the library.

func names tied to PP2 type, eg PP2GCP.ExtractPSCConnectionID, or maybe have tlvparse/{aws,azure,gcp}

Agree, it would be nice to improve the consistency here. I'll think about it for a bit.

@pires
Copy link
Owner

pires commented Sep 8, 2021

Released in https://github.com/pires/go-proxyproto/releases/tag/v0.6.1

@igor-kupczynski igor-kupczynski deleted the gcp-tlv-parser branch September 9, 2021 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants