[GCP] Private Endpoint Support #417

smcclure20 · 2024-07-04T00:01:04Z

[Update] Closed #435 and merged it into this PR. Originally, this PR used the URI of the service attachment for later references of the PSC (ex, adding/removing rules on it). However, since Google services do not have a service attachment, I changed the returned URI to be the forwarding rule that implements the PSC. Users shouldn't even have to use this URI since they can refer to the connection via its name and the tag service will map that to the URI.

Completes the GCP portion of #394

Anticipated UI
For a minimal change to the existing user interface and in alignment with our discussions on the API, this does not require any front-end changes. Instead, users should send requests for private service connections (PSCs) by sending a CreateResource request with the description having one field: "url" which provides the URL of the service attachment to connect to (https://cloud.google.com/vpc/docs/configure-private-service-connect-services)

We can revisit this as needed as we implement for other clouds.

Also in this PR

Some cleanup of resources.go
Creation of a clients struct (like Azure) so that functions do not have to have arguments for each client that may be necessary (this became especially bad as each supported resource requires different -- and potentially many -- clients, but they all have to match the same interface)

Signed-off-by: Sarah McClure <[email protected]>

…point-based implementation Signed-off-by: Sarah McClure <[email protected]>

Signed-off-by: Sarah McClure <[email protected]>

github-actions · 2024-07-04T00:07:44Z

Unit Test Results

309 tests +4 309 ✅ +4 4s ⏱️ +2s
49 suites ±0 0 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit afdaf96. ± Comparison against base commit 9287b6a.

This pull request removes 8 and adds 12 tests. Note that renamed tests count towards both.

github.com/paraglider-project/paraglider/pkg/azure ‑ TestGetVnetsWithMatchingPrefixAddressSpaces
github.com/paraglider-project/paraglider/pkg/azure ‑ TestGetVnetsWithMatchingPrefixAddressSpaces/GetAllVnetsAddressSpaces:_Failure_-_No_Vnet
github.com/paraglider-project/paraglider/pkg/azure ‑ TestGetVnetsWithMatchingPrefixAddressSpaces/GetAllVnetsAddressSpaces:_Failure_-_wrong_name
github.com/paraglider-project/paraglider/pkg/azure ‑ TestGetVnetsWithMatchingPrefixAddressSpaces/GetAllVnetsAddressSpaces:_Success
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestGKECreateWithNetwork
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestGKEFromResourceDecription
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestGKEGetNetworkInfo
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestGKEGetResourceInfo

github.com/paraglider-project/paraglider/pkg/azure ‑ TestGetAllVnetsAddressSpaces
github.com/paraglider-project/paraglider/pkg/azure ‑ TestGetAllVnetsAddressSpaces/GetAllVnetsAddressSpaces:_Failure_-_No_Vnet
github.com/paraglider-project/paraglider/pkg/azure ‑ TestGetAllVnetsAddressSpaces/GetAllVnetsAddressSpaces:_Failure_-_wrong_namespace
github.com/paraglider-project/paraglider/pkg/azure ‑ TestGetAllVnetsAddressSpaces/GetAllVnetsAddressSpaces:_Success
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestClusterCreateWithNetwork
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestClusterFromResourceDecription
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestClusterGetNetworkInfo
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestClusterGetResourceInfo
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestPrivateServiceCreateWithNetwork
github.com/paraglider-project/paraglider/pkg/gcp ‑ TestPrivateServiceGetNetworkInfo
…

♻️ This comment has been updated with latest results.

github-actions · 2024-07-04T00:07:46Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 50.9 %
main branch coverage: 0 %
diff coverage: 50.9 %

The coverage result does not include the functional test results.

Signed-off-by: Sarah McClure <[email protected]>

github-actions · 2024-07-08T23:10:45Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 50.9 %
main branch coverage: 0 %
diff coverage: 50.9 %

The coverage result does not include the functional test results.

Signed-off-by: Sarah McClure <[email protected]>

github-actions · 2024-07-09T00:54:02Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 51.0 %
main branch coverage: 0 %
diff coverage: 51.0 %

The coverage result does not include the functional test results.

github-actions · 2024-07-09T01:51:38Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 51.0 %
main branch coverage: 0 %
diff coverage: 51.0 %

The coverage result does not include the functional test results.

Signed-off-by: Sarah McClure <[email protected]>

github-actions · 2024-07-09T20:47:38Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 51.0 %
main branch coverage: 0 %
diff coverage: 51.0 %

The coverage result does not include the functional test results.

Signed-off-by: Sarah McClure <[email protected]>

Signed-off-by: smcclure20 <[email protected]>

Signed-off-by: Sarah McClure <[email protected]>

github-actions · 2024-07-10T22:08:02Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 51.5 %
main branch coverage: 0 %
diff coverage: 51.5 %

The coverage result does not include the functional test results.

…p-psc-google-services

seankimkdy

Thanks for working on this @smcclure20! Mostly looks good although I'm very new to services in general so it may be helpful for someone like @praveingk or @divega to look at this at a higher level. I left comments mostly about code.

Is it possible to have an integration test for this? Would definitely be helpful to see a barebones example even if it's contrived. But also understand if this is like our k8s example where it's hard to implement an integration test.

pkg/gcp/clients.go

pkg/gcp/plugin_test_utils.go

pkg/gcp/clients.go

pkg/gcp/resources.go

praveingk · 2024-07-15T21:27:33Z

pkg/gcp/resources.go

-	clusterNameFormat = "projects/%s/locations/%s/clusters/%s"
+	clusterTypeName           = "cluster"
+	instanceTypeName          = "instance"
+	serviceAttachmentTypeName = "serviceAttachment"


I find the serviceAttachment as a resource to be confusing, since serviceAttachment is just the target URL of a service, and create function creates a forwarding rule which triggers creation of Private Service Connect endpoints which connects to an existing service attachment url.
Should this be called endpointConnectType to be more specific and common across clouds?

Ah, that's fair. The main reason I kept it as serviceAttachment is due to how it is used in like 142. We have to look at the URL for its type (serviceAttachment) and I thought it was best to be consistent. I could see an argument for renaming the handler though as you mention below.

pkg/gcp/resources.go

praveingk · 2024-07-15T21:38:23Z

pkg/gcp/resources.go

 	} else if err := json.Unmarshal(resourceDesc, createClusterRequest); err == nil && createClusterRequest.Cluster != nil {
-		return &gcpGKE{}, nil
+		return &clusterHandler{}, nil
+	} else if err := json.Unmarshal(resourceDesc, serviceAttachment); err == nil {


Should we check if the request is a proper service URL?

I think we should do type-specific checks like this in getResourceInfo. I'll add that.

praveingk · 2024-07-15T21:56:11Z

pkg/gcp/resources.go

-		return handler, nil
+		handler = &clusterHandler{}
+	} else if resourceType == serviceAttachmentTypeName {
+		handler = &privateServiceHandler{}


Same here. We are returning privateService handler from serviceAttachmentType.

Elaborating a bit more from the above: I think this just reflects the underlying cloud abstractions. We are creating a private service connect, but to do that, we need the URL of a service attachment.

praveingk · 2024-07-15T22:04:55Z

Thanks for adding this support. I have reviewed and posted some suggestions.

Signed-off-by: Sarah McClure <[email protected]>

…non-GCP services Signed-off-by: Sarah McClure <[email protected]>

Signed-off-by: Sarah McClure <[email protected]>

github-actions · 2024-07-17T19:07:29Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 51.5 %
main branch coverage: 0 %
diff coverage: 51.5 %

The coverage result does not include the functional test results.

pkg/gcp/plugin.go

praveingk · 2024-07-22T17:45:48Z

pkg/gcp/resources.go

+	Address     string
+}
+
+type ServiceAttachmentDescription struct {


Since the definition of ServiceAttachmentDescription is defined in paraglider scope and not GCP, may be we need to define it somewhere, when we document private endpoint support. Just noting it here :)

Yeah, we definitely should include it in the docs about this.

Signed-off-by: Sarah McClure <[email protected]>

github-actions · 2024-07-22T19:01:46Z

For the detailed report, please go to Checks tab, click Build and Test, and then download unit_test_coverage artifact at the bottom of build page.

Your PR branch coverage: 51.5 %
main branch coverage: 0 %
diff coverage: 51.5 %

The coverage result does not include the functional test results.

* initial outline of psc support Signed-off-by: Sarah McClure <[email protected]> * created clients struct for convenience and competed switch to PSC endpoint-based implementation Signed-off-by: Sarah McClure <[email protected]> * lint passing and tests mostly written for service support Signed-off-by: Sarah McClure <[email protected]> * most tests written, all passing Signed-off-by: Sarah McClure <[email protected]> * cleanup and add a few tests Signed-off-by: Sarah McClure <[email protected]> * fix target type for rules for service accounts and how URLs are read Signed-off-by: Sarah McClure <[email protected]> * initial setup for google service support Signed-off-by: Sarah McClure <[email protected]> * change workload env variable name to match new actions Signed-off-by: Sarah McClure <[email protected]> * fix firewall naming Signed-off-by: Sarah McClure <[email protected]> * initial implementation; needs testing Signed-off-by: Sarah McClure <[email protected]> * fix build issues Signed-off-by: Sarah McClure <[email protected]> * respond to review comments Signed-off-by: Sarah McClure <[email protected]> * fix tests Signed-off-by: Sarah McClure <[email protected]> * make the paraglider label a constant Signed-off-by: Sarah McClure <[email protected]> --------- Signed-off-by: Sarah McClure <[email protected]> Signed-off-by: smcclure20 <[email protected]> Signed-off-by: Pravein-Govindan-Kannan <[email protected]>

* initial outline of psc support Signed-off-by: Sarah McClure <[email protected]> * created clients struct for convenience and competed switch to PSC endpoint-based implementation Signed-off-by: Sarah McClure <[email protected]> * lint passing and tests mostly written for service support Signed-off-by: Sarah McClure <[email protected]> * most tests written, all passing Signed-off-by: Sarah McClure <[email protected]> * cleanup and add a few tests Signed-off-by: Sarah McClure <[email protected]> * fix target type for rules for service accounts and how URLs are read Signed-off-by: Sarah McClure <[email protected]> * initial setup for google service support Signed-off-by: Sarah McClure <[email protected]> * change workload env variable name to match new actions Signed-off-by: Sarah McClure <[email protected]> * fix firewall naming Signed-off-by: Sarah McClure <[email protected]> * initial implementation; needs testing Signed-off-by: Sarah McClure <[email protected]> * fix build issues Signed-off-by: Sarah McClure <[email protected]> * respond to review comments Signed-off-by: Sarah McClure <[email protected]> * fix tests Signed-off-by: Sarah McClure <[email protected]> * make the paraglider label a constant Signed-off-by: Sarah McClure <[email protected]> --------- Signed-off-by: Sarah McClure <[email protected]> Signed-off-by: smcclure20 <[email protected]> Signed-off-by: Julian Tweneboa Kodua <[email protected]> Signed-off-by: Julian Tweneboa Kodua <[email protected]>

smcclure20 added 5 commits July 1, 2024 17:10

initial outline of psc support

1558d5b

Signed-off-by: Sarah McClure <[email protected]>

created clients struct for convenience and competed switch to PSC end…

5686cb9

…point-based implementation Signed-off-by: Sarah McClure <[email protected]>

lint passing and tests mostly written for service support

4f98219

Signed-off-by: Sarah McClure <[email protected]>

most tests written, all passing

d5bdfa4

Signed-off-by: Sarah McClure <[email protected]>

cleanup and add a few tests

935a55a

Signed-off-by: Sarah McClure <[email protected]>

smcclure20 requested a review from a team as a code owner July 4, 2024 00:01

smcclure20 marked this pull request as draft July 4, 2024 00:01

fix target type for rules for service accounts and how URLs are read

ef17191

Signed-off-by: Sarah McClure <[email protected]>

smcclure20 temporarily deployed to functional-tests July 8, 2024 23:04 — with GitHub Actions Inactive

smcclure20 marked this pull request as ready for review July 8, 2024 23:04

smcclure20 mentioned this pull request Jul 8, 2024

[GCP] Support Google Services for PSCs #426

Closed

smcclure20 added 2 commits July 8, 2024 17:46

initial setup for google service support

a8dbe00

Signed-off-by: Sarah McClure <[email protected]>

change workload env variable name to match new actions

001b021

Signed-off-by: Sarah McClure <[email protected]>

smcclure20 temporarily deployed to functional-tests July 9, 2024 00:47 — with GitHub Actions Inactive

Merge branch 'main' into smcclure20/private-endpoint-support

b0e9777

smcclure20 temporarily deployed to functional-tests July 9, 2024 01:45 — with GitHub Actions Inactive

fix firewall naming

82414fb

Signed-off-by: Sarah McClure <[email protected]>

smcclure20 temporarily deployed to functional-tests July 9, 2024 20:40 — with GitHub Actions Inactive

smcclure20 and others added 2 commits July 10, 2024 12:34

initial implementation; needs testing

54e5e13

Signed-off-by: Sarah McClure <[email protected]>

Merge branch 'main' into smcclure20/private-endpoint-support

fbfe3b0

Signed-off-by: smcclure20 <[email protected]>

smcclure20 temporarily deployed to functional-tests July 10, 2024 21:31 — with GitHub Actions Inactive

fix build issues

54934b1

Signed-off-by: Sarah McClure <[email protected]>

smcclure20 temporarily deployed to functional-tests July 10, 2024 22:01 — with GitHub Actions Inactive

Merge branch 'smcclure20/private-endpoint-support' into smcclure20/gc…

86b20cc

…p-psc-google-services

smcclure20 mentioned this pull request Jul 11, 2024

[GCP] Private connect support for Google services #435

Closed

seankimkdy requested changes Jul 12, 2024

View reviewed changes

praveingk requested changes Jul 15, 2024

View reviewed changes

respond to review comments

6cacecd

Signed-off-by: Sarah McClure <[email protected]>

smcclure20 had a problem deploying to functional-tests July 16, 2024 20:51 — with GitHub Actions Failure

smcclure20 added 2 commits July 16, 2024 16:04

refactored around forwarding rule URLs (which exist for both GCP and …

5aa2569

…non-GCP services Signed-off-by: Sarah McClure <[email protected]>

fix tests

34e37f5

Signed-off-by: Sarah McClure <[email protected]>

smcclure20 temporarily deployed to functional-tests July 17, 2024 19:01 — with GitHub Actions Inactive

smcclure20 requested review from praveingk and seankimkdy July 17, 2024 22:53

seankimkdy mentioned this pull request Jul 22, 2024

[Azure] Add NAT gateways for outbound public IP addresses #441

Merged

seankimkdy requested changes Jul 22, 2024

View reviewed changes

pkg/gcp/plugin.go Outdated Show resolved Hide resolved

pkg/gcp/plugin.go Outdated Show resolved Hide resolved

praveingk reviewed Jul 22, 2024

View reviewed changes

praveingk approved these changes Jul 22, 2024

View reviewed changes

make the paraglider label a constant

afdaf96

Signed-off-by: Sarah McClure <[email protected]>

smcclure20 temporarily deployed to functional-tests July 22, 2024 18:54 — with GitHub Actions Inactive

smcclure20 requested a review from seankimkdy July 22, 2024 18:54

seankimkdy approved these changes Jul 22, 2024

View reviewed changes

smcclure20 merged commit e61c0ba into main Jul 23, 2024
10 checks passed

smcclure20 deleted the smcclure20/private-endpoint-support branch July 23, 2024 16:39

divega mentioned this pull request Jul 24, 2024

Support for service resources using virtual private endpoints, public access, and service-to-service controls #344

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GCP] Private Endpoint Support #417

[GCP] Private Endpoint Support #417

smcclure20 commented Jul 4, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 10, 2024

seankimkdy left a comment

praveingk Jul 15, 2024

smcclure20 Jul 16, 2024

praveingk Jul 15, 2024

smcclure20 Jul 16, 2024

praveingk Jul 15, 2024

smcclure20 Jul 16, 2024

praveingk commented Jul 15, 2024

github-actions bot commented Jul 17, 2024

praveingk Jul 22, 2024

smcclure20 Jul 22, 2024

github-actions bot commented Jul 22, 2024

[GCP] Private Endpoint Support #417

[GCP] Private Endpoint Support #417

Conversation

smcclure20 commented Jul 4, 2024 • edited Loading

github-actions bot commented Jul 4, 2024 • edited Loading

Unit Test Results

github-actions bot commented Jul 4, 2024

github-actions bot commented Jul 8, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 9, 2024

github-actions bot commented Jul 10, 2024

seankimkdy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

praveingk commented Jul 15, 2024

github-actions bot commented Jul 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 22, 2024

smcclure20 commented Jul 4, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024 •

edited

Loading