BYO VPC Validation #2493

patrickdillon · 2019-10-10T18:16:57Z

Jira: CORS-1216

This PR:

creates a client to make calls to the GCP API
validates user-provided VPC networks and subnets
validates the install config for required fields when providing VPC & subnets
creates tests for user-provided VPC scenario
updates existing DNS code to use the client

pkg/asset/installconfig/gcp/client.go

pkg/asset/installconfig/gcp/validation.go

pkg/asset/installconfig/gcp/client.go

pkg/asset/installconfig/gcp/validation.go

abhinavdahiya · 2019-10-14T18:17:09Z

/test e2e-gcp

patrickdillon · 2019-10-14T19:26:44Z

/test e2e-gcp

abhinavdahiya · 2019-10-14T20:47:28Z

/test e2e-gcp

patrickdillon · 2019-10-14T22:45:29Z

/test e2e-gcp

pkg/asset/installconfig/gcp/validation.go

abhinavdahiya · 2019-10-14T23:12:54Z

pkg/asset/installconfig/gcp/validation.go

Also it seems like this function should be called validatePlatform as it does validation for VPC + subnets.

maybe validatePlatform(client API, platform *types.GCPPlatform, networking *types.Networking, path *field.Path) field.ErrorList

I agree we should change the name from VPC, but I imagined the top level validate function to be the equivalent of validatePlatform. If there were further platform-specific data that needed to be added they would not be added here. Perhaps validateNetwork or validateNetworks?

pkg/asset/installconfig/gcp/validation.go

This creates an interface and a wrapper for calls to the GCP API. This initial implementation has a bare-bones method for retrieiving VPC networks. This will be important for testing so that we can stub out calls to the GCP API.

patrickdillon · 2019-10-15T15:48:13Z

/test e2e-gcp

pkg/asset/installconfig/gcp/validation.go

pkg/asset/installconfig/installconfig.go

pkg/asset/mock/gcpclient_generated.go

Adds logic to check Google API to make sure user-provided VPC information is valid. Checks that the VPC belongs to project, subnets belong to VPC, and subnets are in the correct region.

This generates mocks for the GCP client API interface and creates tests for the BYO VPC validation use case.

Checks to make sure both subnets are required in install config if a network is specified.

Rewrites existing DNS code for getting basedomains and dns public zone to utliize new gcp client. Regenerate mock with addition to interface.

patrickdillon · 2019-10-15T19:33:28Z

/test e2e-gcp

patrickdillon · 2019-10-15T19:36:23Z

I added 6cf9eda to incorporate DNS to use the client. I think it seems like a natural fit, but not sure if we want to include it. I started to write tests but the scenarios seem pretty artificial, so I think it is ok without them.

abhinavdahiya · 2019-10-15T22:56:28Z

/retest

abhinavdahiya · 2019-10-15T22:56:35Z

/approve

abhinavdahiya · 2019-10-16T01:29:58Z

/lgtm

openshift-ci-robot · 2019-10-16T01:30:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, patrickdillon

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [abhinavdahiya]

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

openshift-bot · 2019-10-16T02:35:04Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 10, 2019

openshift-ci-robot requested review from jcpowermac and jhixson74 October 10, 2019 18:17

patrickdillon force-pushed the byo-vpc-validation branch from 3dcf74b to 6953ab7 Compare October 10, 2019 19:55

abhinavdahiya reviewed Oct 10, 2019

View reviewed changes

pkg/asset/installconfig/gcp/client.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Oct 10, 2019

View reviewed changes

pkg/asset/installconfig/gcp/validation.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Oct 10, 2019

View reviewed changes

pkg/asset/installconfig/gcp/validation.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Oct 10, 2019

View reviewed changes

pkg/asset/installconfig/gcp/client.go Outdated Show resolved Hide resolved

patrickdillon force-pushed the byo-vpc-validation branch 5 times, most recently from b7ba2ff to 8b26c38 Compare October 14, 2019 01:51

patrickdillon changed the title ~~WIP: BYO VPC Validation~~ BYO VPC Validation Oct 14, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 14, 2019

abhinavdahiya reviewed Oct 14, 2019

View reviewed changes

pkg/asset/installconfig/gcp/validation.go Outdated Show resolved Hide resolved

patrickdillon force-pushed the byo-vpc-validation branch from 8b26c38 to 59db9a4 Compare October 14, 2019 19:26

patrickdillon force-pushed the byo-vpc-validation branch 2 times, most recently from 486b6ca to 13a7128 Compare October 14, 2019 19:37

abhinavdahiya reviewed Oct 14, 2019

View reviewed changes

pkg/asset/installconfig/gcp/validation.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Oct 14, 2019

View reviewed changes

pkg/asset/installconfig/gcp/validation.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Oct 14, 2019

View reviewed changes

pkg/asset/installconfig/gcp/validation.go Outdated Show resolved Hide resolved

patrickdillon force-pushed the byo-vpc-validation branch 2 times, most recently from c431991 to 72695ae Compare October 15, 2019 00:59

patrickdillon force-pushed the byo-vpc-validation branch from 2624fe0 to a7bcdc0 Compare October 15, 2019 14:26

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 15, 2019

Create client for calls to GCP API.

c82b364

This creates an interface and a wrapper for calls to the GCP API. This initial implementation has a bare-bones method for retrieiving VPC networks. This will be important for testing so that we can stub out calls to the GCP API.

patrickdillon force-pushed the byo-vpc-validation branch from a7bcdc0 to 6f12422 Compare October 15, 2019 14:29

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 15, 2019

patrickdillon force-pushed the byo-vpc-validation branch from 6f12422 to 40d19fb Compare October 15, 2019 14:42

abhinavdahiya reviewed Oct 15, 2019

View reviewed changes

pkg/asset/installconfig/gcp/validation.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Oct 15, 2019

View reviewed changes

pkg/asset/installconfig/installconfig.go Outdated Show resolved Hide resolved

abhinavdahiya reviewed Oct 15, 2019

View reviewed changes

pkg/asset/mock/gcpclient_generated.go Outdated Show resolved Hide resolved

Add platform-specific validation for BYO VPC.

bcc9898

Adds logic to check Google API to make sure user-provided VPC information is valid. Checks that the VPC belongs to project, subnets belong to VPC, and subnets are in the correct region.

patrickdillon force-pushed the byo-vpc-validation branch from 40d19fb to c81f353 Compare October 15, 2019 16:50

patrickdillon added 3 commits October 15, 2019 15:02

Add mocks and tests for GCP BYO VPC validation.

7b3e193

This generates mocks for the GCP client API interface and creates tests for the BYO VPC validation use case.

Add required field validation for GCP BYO VPC.

1cdb136

Checks to make sure both subnets are required in install config if a network is specified.

Update GCP DNS services to use client.

6cf9eda

Rewrites existing DNS code for getting basedomains and dns public zone to utliize new gcp client. Regenerate mock with addition to interface.

patrickdillon force-pushed the byo-vpc-validation branch from c81f353 to 6cf9eda Compare October 15, 2019 19:30

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 15, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 15, 2019

openshift-ci-robot assigned abhinavdahiya Oct 16, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 16, 2019

openshift-merge-robot merged commit af56855 into openshift:master Oct 16, 2019

patrickdillon deleted the byo-vpc-validation branch October 24, 2019 18:22

BYO VPC Validation #2493

BYO VPC Validation #2493

Uh oh!

Conversation

patrickdillon commented Oct 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abhinavdahiya commented Oct 14, 2019

Uh oh!

patrickdillon commented Oct 14, 2019

Uh oh!

abhinavdahiya commented Oct 14, 2019

Uh oh!

patrickdillon commented Oct 14, 2019

Uh oh!

Uh oh!

abhinavdahiya Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickdillon Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

patrickdillon commented Oct 15, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

patrickdillon commented Oct 15, 2019

Uh oh!

patrickdillon commented Oct 15, 2019

Uh oh!

abhinavdahiya commented Oct 15, 2019

Uh oh!

abhinavdahiya commented Oct 15, 2019

Uh oh!

abhinavdahiya commented Oct 16, 2019

Uh oh!

openshift-ci-robot commented Oct 16, 2019

Uh oh!

openshift-bot commented Oct 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

patrickdillon commented Oct 10, 2019 •

edited

Loading

abhinavdahiya Oct 14, 2019 •

edited

Loading