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

feat(networkconnectivity): add Service Connection Policy support #624

Conversation

rickard-von-essen
Copy link
Contributor

@rickard-von-essen rickard-von-essen commented Sep 26, 2024

Description of your changes

This adds support for google_network_connectivity_service_connection_policy Terraform resource, see Terraform - Google - Service Connection Policy.

Includes #631

I have:

  • Read and followed Crossplane's [contribution process].
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Via uptest: https://github.com/crossplane-contrib/provider-upjet-gcp/actions/runs/11480002642

@rickard-von-essen
Copy link
Contributor Author

Working on a fix for this in upjet, here rickard-von-essen/upjet @ bug/ref-subnet, see this line.

@rickard-von-essen
Copy link
Contributor Author

Depends on fix included in #631

@turkenf
Copy link
Collaborator

turkenf commented Oct 9, 2024

Depends on fix included in #631

It might be better to test the Upjet update in this PR.

@rickard-von-essen
Copy link
Contributor Author

Depends on fix included in #631

It might be better to test the Upjet update in this PR.

Sure I'll rebase this onto that. brb

@rickard-von-essen rickard-von-essen force-pushed the feature/service-connection-policy branch from 0bd131b to ac1905f Compare October 12, 2024 18:36
@rickard-von-essen rickard-von-essen marked this pull request as ready for review October 12, 2024 18:38
@rickard-von-essen
Copy link
Contributor Author

Rebased onto #631

@turkenf
Copy link
Collaborator

turkenf commented Oct 16, 2024

/test-examples="examples/networkconnectivity/v1beta1/serviceconnectionpolicy.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.

Thank you for your effort @rickard-von-essen, left two comments for you to consider.

config/networkconnectivity/config.go Outdated Show resolved Hide resolved
@turkenf
Copy link
Collaborator

turkenf commented Oct 21, 2024

/test-examples="examples/networkconnectivity/v1beta1/serviceconnectionpolicy.yaml"

1 similar comment
@turkenf
Copy link
Collaborator

turkenf commented Oct 22, 2024

/test-examples="examples/networkconnectivity/v1beta1/serviceconnectionpolicy.yaml"

@rickard-von-essen
Copy link
Contributor Author

Great! Very happy to finally get this to the finishing line. This was a bit rough learning curve for me. 🫠

@turkenf
Copy link
Collaborator

turkenf commented Oct 22, 2024

Thank you for your interest and effort @rickard-von-essen.

Apart from my question above, in order to move this PR forward, I will need to take over the upjet upgrade PR and make some tests and additions there. I see changes in the APIs files of many resources, I will check them separately in the upjet upgrade PR and make the necessary additions/changes (will add the new ref fields from v1beta2 to v1beta1 apis). After merging the upgrade PR, we can rebase this PR and move it forward.

@rickard-von-essen
Copy link
Contributor Author

[...] in order to move this PR forward, I will need to take over the upjet upgrade #631 and make some tests and additions there.

Sounds great! Thanks for the support and please ping me there if there are any questions or anything I help out with.

@turkenf
Copy link
Collaborator

turkenf commented Oct 23, 2024

The upjet upgrade PR is merged @rickard-von-essen. Could you please rebase your branch to the main and squash your commits to make the commit history clean?

This adds support for google_network_connectivity_service_connection_policy Terraform resource, see
https://registry.terraform.io/providers/hashicorp/google/5.39.1/docs/resources/network_connectivity_service_connection_policy

Signed-off-by: Rickard von Essen <rickard.von.essen@gmail.com>
Signed-off-by: Rickard von Essen <rickard.von.essen@gmail.com>
Signed-off-by: Rickard von Essen <rickard.von.essen@gmail.com>
Signed-off-by: Rickard von Essen <rickard.von.essen@gmail.com>
Signed-off-by: Rickard von Essen <rickard.von.essen@gmail.com>
Signed-off-by: Rickard von Essen <rickard.von.essen@gmail.com>
@rickard-von-essen rickard-von-essen force-pushed the feature/service-connection-policy branch from b1c12db to 5ce160a Compare October 23, 2024 12:12
@turkenf
Copy link
Collaborator

turkenf commented Oct 23, 2024

/test-examples="examples/networkconnectivity/v1beta1/serviceconnectionpolicy.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.

Many thanks @rickard-von-essen, LGTM 🙌

@turkenf turkenf merged commit 7344301 into crossplane-contrib:main Oct 23, 2024
10 checks passed
@rickard-von-essen rickard-von-essen deleted the feature/service-connection-policy branch October 26, 2024 11:22
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.

2 participants