Firewallrules creation

[barbacbd]

What type of PR is this?

/kind feature /kind /api-change

What this PR does / why we need it:

Currently the basic/default/required firewall rules are created by CAPG. Users should be given the ability to create the firewall rules associated with VPC that CAPG will create.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

• [x] squashed commits
• [ ] includes documentation
• [x] adds unit tests

Release note:

Currently the basic/default/required firewall rules are created by CAPG.
Users should be given the ability to create the firewall rules associated with
VPC that CAPG will create. The information provided by the user will mirror the
parameters for compute.Firewalls.

[k8s-ci-robot]

@barbacbd: The label(s) kind//api-change cannot be applied, because the repository doesn't have them.

In response to this:

What type of PR is this?

/kind feature /kind /api-change

What this PR does / why we need it:

Currently the basic/default/required firewall rules are created by CAPG. Users should be given the ability to create the firewall rules associated with VPC that CAPG will create.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

• [x] squashed commits
• [ ] includes documentation
• [x] adds unit tests

Release note:

Currently the basic/default/required firewall rules are created by CAPG.
Users should be given the ability to create the firewall rules associated with
VPC that CAPG will create. The information provided by the user will mirror the
parameters for compute.Firewalls.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-ci-robot]

Hi @barbacbd. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[netlify[bot]]

✅ Deploy Preview for kubernetes-sigs-cluster-api-gcp ready!

Name	Link
🔨 Latest commit	95f8eb6593600f84d85242b5b2273e41fd2d14c7
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-cluster-api-gcp/deploys/69330eec32fd5c0008dbb682
😎 Deploy Preview	https://deploy-preview-1538--kubernetes-sigs-cluster-api-gcp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[barbacbd]

Note this currently requires https://github.com/kubernetes-sigs/cluster-api-provider-gcp/pull/1532 so the commits look a bit wonky. These will be removed when 1532 merges and it should look cleaner.

[justinsb]

I like this from a functionality and code perspective. Does comparable functionality already exist in (for example) CAPA? If so, we're just "catching up" and that's great, if not we should probably discuss at the CAPI level to make sure we want to include this functionality here.

As an alternative, I believe CAPZ lets you define additional resources using ASO. The GCP equivalent is KCC (and the AWS equivalent in ACK). I work on KCC so I am obviously biased here, but it is a good way to avoid having to reimplement (potentially) every GCP API in our CAPG API.

[patrickdillon]

I like this from a functionality and code perspective. Does comparable functionality already exist in (for example) CAPA? If so, we're just "catching up" and that's great

Yes! CAPA allows you to specify CNIIngressRules and IngressRules for ControlPlane and Compute nodes

[mloiseleur]

I'm not a maintainer of this project, but I'm very interested in it. So I'll enable the test as a first step. Many thanks @barbacbd :+1: ! /ok-to-test

[barbacbd]

@darkweaver87 There was some discussion about this in https://github.com/kubernetes-sigs/cluster-api-provider-gcp/pull/1532. Instead of UserManaged the user should provide no firewall rules AND they should also set to Unmanaged. It is assumed that when no firewall rules are provided that the user either 1) does not want any or 2) created them on their own. Managed/Unmanaged will Only apply to the default rules that CAPG would usually create.

[darkweaver87]

Thanks @barbacbd for the info. I can understand the direction but then it should be possible to have the same "bring your own infrastructure" mode as AWS provider does. My proposition was just a quick win because providing all the infrastructure is another story I suppose :-)

[barbacbd]

Thanks @barbacbd for the info. I can understand the direction but then it should be possible to have the same "bring your own infrastructure" mode as AWS provider does. My proposition was just a quick win because providing all the infrastructure is another story I suppose :-)

@darkweaver87 I think that this can be a start to a bring your own infrastructure. This is a change to help the downstream project. I think its a good start that maybe we should open an issue for a complete BYOI. What do you think?

[k8s-ci-robot]

@darkweaver87: changing LGTM is restricted to collaborators

In response to this:

LGTM

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: barbacbd, darkweaver87 Once this PR has been reviewed and has the lgtm label, please assign salasberryfin for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

@barbacbd: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-gcp-apidiff	95f8eb6593600f84d85242b5b2273e41fd2d14c7	link	false	/test pull-cluster-api-provider-gcp-apidiff

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[damdo]

/assign @JoelSpeed As you mentioned you wanted to look at it API wise