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.

Sign up for GitHub

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

Jump to bottom

feat: add webhook validation #62

Merged
merged 1 commit into from
Apr 22, 2022
Merged

feat: add webhook validation #62

merged 1 commit into from
Apr 22, 2022

Conversation

joekr
Copy link
Member

@joekr joekr commented Apr 14, 2022

What this PR does / why we need it:
Validation webhooks will make sure that mandatory parameters are set before reconciliation so that users can catch errors much earlier.

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 #55

@joekr joekr added the enhancement New feature or request label Apr 14, 2022
@joekr joekr self-assigned this Apr 14, 2022
@joekr joekr force-pushed the issue-55-validation-webhooks branch from c7b1eca to 5c4c05d Compare April 14, 2022 18:10
@joekr
Copy link
Member Author

joekr commented Apr 15, 2022 •
edited
Loading

unit tests

?       github.com/oracle/cluster-api-provider-oci      [no test files]
ok      github.com/oracle/cluster-api-provider-oci/api/v1beta1  0.301s  coverage: 7.5% of statements
?       github.com/oracle/cluster-api-provider-oci/cloud/config [no test files]
ok      github.com/oracle/cluster-api-provider-oci/cloud/ociutil        0.579s  coverage: 30.3% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/scope  176.018s        coverage: 81.9% of statements
?       github.com/oracle/cluster-api-provider-oci/cloud/scope/mocks    [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/compute       [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/compute/mock_compute  [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/identity      [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/identity/mock_identity        [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer   [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer/mock_nlb  [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/vcn   [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/vcn/mock_vcn  [no test files]
ok      github.com/oracle/cluster-api-provider-oci/controllers  26.312s coverage: 70.3% of statements

e2e tests

Summarizing 1 Failure:

[Fail] Workload cluster creation [It] Cloud Provider OCI testing
/home/ubuntu/projects/cluster-api-provider-oci/test/e2e/ccm_helpers.go:83

Ran 9 of 20 Specs in 2851.512 seconds
FAIL! -- 8 Passed | 1 Failed | 0 Pending | 11 Skipped

Cloud Provider OCI testing passed when running against SYD

• [SLOW TEST:985.010 seconds]
Workload cluster creation
/home/ubuntu/projects/cluster-api-provider-oci/test/e2e/cluster_test.go:49
  Cloud Provider OCI testing
  /home/ubuntu/projects/cluster-api-provider-oci/test/e2e/cluster_test.go:201
------------------------------
STEP: Tearing down the management cluster


Ran 1 of 20 Specs in 1084.876 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 19 Skipped

@joekr joekr force-pushed the issue-55-validation-webhooks branch 2 times, most recently from c231467 to d421e8d Compare April 21, 2022 13:04
@joekr joekr marked this pull request as ready for review April 21, 2022 23:27
@joekr joekr force-pushed the issue-55-validation-webhooks branch from d421e8d to c417311 Compare April 21, 2022 23:28
shyamradhakrishnan
shyamradhakrishnan reviewed Apr 22, 2022
View reviewed changes
api/v1beta1/ocicluster_webhook.go Outdated Show resolved Hide resolved
api/v1beta1/webhooks.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
test/e2e/data/infrastructure-oci/v1beta1/cluster-template-cluster-class.yaml Outdated Show resolved Hide resolved
@joekr joekr force-pushed the issue-55-validation-webhooks branch from c417311 to 80602d3 Compare April 22, 2022 11:31
@joekr joekr force-pushed the issue-55-validation-webhooks branch from 80602d3 to f459f94 Compare April 22, 2022 11:42
@joekr
Copy link
Member Author

joekr commented Apr 22, 2022

updated tests after rebase and changes

?       github.com/oracle/cluster-api-provider-oci      [no test files]
ok      github.com/oracle/cluster-api-provider-oci/api/v1beta1  0.305s  coverage: 7.6% of statements
?       github.com/oracle/cluster-api-provider-oci/cloud/config [no test files]
ok      github.com/oracle/cluster-api-provider-oci/cloud/ociutil        0.602s  coverage: 30.3% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/scope  176.026s        coverage: 81.9% of statements
?       github.com/oracle/cluster-api-provider-oci/cloud/scope/mocks    [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/compute       [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/compute/mock_compute  [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/identity      [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/identity/mock_identity        [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer   [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer/mock_nlb  [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/vcn   [no test files]
?       github.com/oracle/cluster-api-provider-oci/cloud/services/vcn/mock_vcn  [no test files]
ok      github.com/oracle/cluster-api-provider-oci/controllers  26.240s coverage: 70.7% of statements

Summarizing 1 Failure:

[Fail] Workload cluster creation [It] Cloud Provider OCI testing [PRBlocking]
/home/ubuntu/projects/cluster-api-provider-oci/test/e2e/ccm_helpers.go:83

Ran 5 of 21 Specs in 1872.951 seconds
FAIL! -- 4 Passed | 1 Failed | 0 Pending | 16 Skipped

testing in SYD for CCM

• [SLOW TEST:1075.668 seconds]
Workload cluster creation
/home/ubuntu/projects/cluster-api-provider-oci/test/e2e/cluster_test.go:49
  Cloud Provider OCI testing [PRBlocking]
  /home/ubuntu/projects/cluster-api-provider-oci/test/e2e/cluster_test.go:201
------------------------------
STEP: Tearing down the management cluster


Ran 1 of 21 Specs in 1160.600 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 20 Skipped

@joekr joekr merged commit 46ad859 into oracle:main Apr 22, 2022
@joekr joekr deleted the issue-55-validation-webhooks branch April 26, 2022 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shyamradhakrishnan shyamradhakrishnan shyamradhakrishnan approved these changes

@arindam-bandyopadhyay arindam-bandyopadhyay Awaiting requested review from arindam-bandyopadhyay

Assignees

@joekr joekr

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add validations using validation webhooks
2 participants
@joekr @shyamradhakrishnan