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 google_compute_region_target_tcp_proxy gcp resource #432

Merged

Conversation

meerkat-b
Copy link
Contributor

@meerkat-b meerkat-b commented Dec 13, 2023

Description of your changes

This change adds support of the google_compute_region_target_tcp_proxy resource to the gcp provider.

Fixes #433

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Tested locally by applying the crds, controllers, and examples to local kind cluster:
image

  • Requesting the aid of contributors to help me trigger the uptest CI workflow with the command:
    /test-examples="examples/compute/regiontargettcpproxy.yaml"

Thanks!

@Upbound-CLA
Copy link

Upbound-CLA commented Dec 13, 2023

CLA assistant check
All committers have signed the CLA.

@meerkat-b meerkat-b force-pushed the add_region_target_tcp_proxy_resource branch from c3383f2 to 559799f Compare December 13, 2023 11:14
@meerkat-b
Copy link
Contributor Author

meerkat-b commented Dec 13, 2023

/test-examples="provider-aws/examples/compute/regiontargettcpproxy.yaml"
Erroneously copied from the documentation, requesting someone to comment with
/test-examples="examples/compute/regiontargettcpproxy.yaml"

@jeanduplessis
Copy link
Collaborator

/test-examples="provider-aws/examples/compute/regiontargettcpproxy.yaml"

@meerkat-b meerkat-b changed the title Add support for region_target_tcp_proxy gcp resource Add support for google_compute_region_target_tcp_proxy gcp resource Dec 13, 2023
@meerkat-b meerkat-b force-pushed the add_region_target_tcp_proxy_resource branch from 559799f to a5531c1 Compare December 13, 2023 12:07
@meerkat-b
Copy link
Contributor Author

meerkat-b commented Dec 13, 2023

/test-examples="provider-aws/examples/compute/regiontargettcpproxy.yaml"
Erroneously copied from the documentation, requesting someone to comment with
/test-examples="examples/compute/regiontargettcpproxy.yaml"

@jeanduplessis
Copy link
Collaborator

/test-examples="provider-aws/examples/compute/regiontargettcpproxy.yaml"

@jeanduplessis
Copy link
Collaborator

/test-examples="examples/compute/regiontargettcpproxy.yaml"

@meerkat-b meerkat-b marked this pull request as ready for review December 13, 2023 13:54
@jeanduplessis
Copy link
Collaborator

/test-examples="examples/compute/regiontargettcpproxy.yaml"

Copy link
Collaborator

@turkenf turkenf left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR @meerkat-b, I left two small comments for you to consider.

config/compute/config.go Outdated Show resolved Hide resolved
config/externalnamenottested.go Outdated Show resolved Hide resolved
@meerkat-b meerkat-b force-pushed the add_region_target_tcp_proxy_resource branch from b7d6ced to eafd8dd Compare December 14, 2023 10:38
@meerkat-b
Copy link
Contributor Author

@turkenf thanks for your comments.
I've applied the two suggestions, rebased against main, and also re-tested after changes against my local kind cluster.

image

@turkenf
Copy link
Collaborator

turkenf commented Dec 14, 2023

/test-examples="examples/compute/regiontargettcpproxy.yaml"

Copy link
Collaborator

@turkenf turkenf left a comment

Choose a reason for hiding this comment

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

Thanks for your nice work @meerkat-b, LGTM.

@turkenf turkenf merged commit fc25bf4 into crossplane-contrib:main Dec 14, 2023
10 checks passed
@meerkat-b
Copy link
Contributor Author

thanks for the prompt review!

@meerkat-b meerkat-b deleted the add_region_target_tcp_proxy_resource branch December 14, 2023 13:40
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.

Request for compute_region_target_tcp_proxy resource
4 participants