Skip to content

Conversation

joekr
Copy link
Member

@joekr joekr commented Oct 9, 2025

What this PR does / why we need it:
The API documentation points out both Destination and Protocol are required https://docs.oracle.com/en-us/iaas/api/#/en/iaas/20160918/datatypes/EgressSecurityRule but our validator doesn't catch those. This also handles the IngressSecurityRule https://docs.oracle.com/en-us/iaas/api/#/en/iaas/20160918/datatypes/IngressSecurityRule Source and Protocol that is required.

This does also add a few new unit tests to make sure we cover nil checks.

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

@joekr joekr self-assigned this Oct 9, 2025
@joekr joekr added the bug Something isn't working label Oct 9, 2025
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Oct 9, 2025
@joekr
Copy link
Member Author

joekr commented Oct 9, 2025

unit tests

        github.com/oracle/cluster-api-provider-oci              coverage: 0.0% of statements
ok      github.com/oracle/cluster-api-provider-oci/api/v1beta1  26.316s coverage: 23.3% of statements
ok      github.com/oracle/cluster-api-provider-oci/api/v1beta2  1.147s  coverage: 15.1% of statements
        github.com/oracle/cluster-api-provider-oci/cloud/config         coverage: 0.0% of statements
        github.com/oracle/cluster-api-provider-oci/cloud/metrics                coverage: 0.0% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/ociutil        0.829s  coverage: 15.9% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/ociutil/ptr    0.848s  coverage: 100.0% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/scope  20.901s coverage: 74.4% of statements
        github.com/oracle/cluster-api-provider-oci/cloud/scope/mocks            coverage: 0.0% of statements
        github.com/oracle/cluster-api-provider-oci/cloud/services/base          coverage: 0.0% of statements
        github.com/oracle/cluster-api-provider-oci/cloud/services/base/mock_base                coverage: 0.0% of statements
?       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          coverage: 0.0% of statements
?       github.com/oracle/cluster-api-provider-oci/cloud/services/computemanagement     [no test files]
        github.com/oracle/cluster-api-provider-oci/cloud/services/computemanagement/mock_computemanagement              coverage: 0.0% of statements
?       github.com/oracle/cluster-api-provider-oci/cloud/services/containerengine       [no test files]
        github.com/oracle/cluster-api-provider-oci/cloud/services/containerengine/mock_containerengine          coverage: 0.0% of statements
?       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                coverage: 0.0% of statements
?       github.com/oracle/cluster-api-provider-oci/cloud/services/loadbalancer  [no test files]
        github.com/oracle/cluster-api-provider-oci/cloud/services/loadbalancer/mock_lb          coverage: 0.0% of statements
?       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          coverage: 0.0% of statements
?       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          coverage: 0.0% of statements
?       github.com/oracle/cluster-api-provider-oci/cloud/services/workrequests  [no test files]
        github.com/oracle/cluster-api-provider-oci/cloud/services/workrequests/mock_workrequests                coverage: 0.0% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/util   1.332s  coverage: 60.5% of statements
ok      github.com/oracle/cluster-api-provider-oci/controllers  2.895s  coverage: 59.2% of statements
ok      github.com/oracle/cluster-api-provider-oci/exp/api/v1beta1      7.419s  coverage: 15.3% of statements
ok      github.com/oracle/cluster-api-provider-oci/exp/api/v1beta2      1.786s  coverage: 5.8% of statements
ok      github.com/oracle/cluster-api-provider-oci/exp/controllers      2.484s  coverage: 56.3% of statements
        github.com/oracle/cluster-api-provider-oci/feature              coverage: 0.0% of statements
?       github.com/oracle/cluster-api-provider-oci/version      [no test files]

e2e tests

Summarizing 2 Failures:
  [FAIL] Managed Workload cluster creation [It] Managed Cluster - Virtual Node Pool [PRBlocking]
  /home/ubuntu/go/pkg/mod/sigs.k8s.io/cluster-api/test@v1.10.6/framework/clusterctl/clusterctl_helpers.go:431
  [FAIL] Managed Workload cluster creation [It] Managed Cluster - Simple [PRBlocking]
  /home/ubuntu/go/pkg/mod/sigs.k8s.io/cluster-api/test@v1.10.6/framework/clusterctl/clusterctl_helpers.go:431

Ran 9 of 31 Specs in 2087.279 seconds
FAIL! -- 7 Passed | 2 Failed | 0 Pending | 22 Skipped
  • Look into why the managed clusters are failing with egressrules

Tests after editing the managed webhook

Ran 9 of 31 Specs in 3853.702 seconds
SUCCESS! -- 9 Passed | 0 Failed | 0 Pending | 22 Skipped

@joekr
Copy link
Member Author

joekr commented Oct 10, 2025

After last code update all tests still passing.

The API documentation points out both Destination and Protocol are
required https://docs.oracle.com/en-us/iaas/api/#/en/iaas/20160918/datatypes/EgressSecurityRule
but our validator doesn't catch those.
Copy link
Member

@vladcristi vladcristi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from my point of view

@joekr joekr merged commit 1cdd003 into oracle:main Oct 13, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working OCA Verified All contributors have signed the Oracle Contributor Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NSG EgressSecurityRule shouldn't allow missing required parameters

2 participants