Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion osac-operator/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,12 +514,16 @@ func setupNetworkingControllers(
// nil otherwise, in which case those controllers always use the legacy
// implementation-strategy path.
var resolver *dispatcher.Resolver
var networkClassesClient privatev1.NetworkClassesClient

if grpcConn != nil && networkingNamespace != "" {
networkClassesClient = privatev1.NewNetworkClassesClient(grpcConn)

disc, err := networkmanager.NewDiscovery(localMgr.GetClient(), networkingNamespace)
if err != nil {
return fmt.Errorf("network manager discovery: %w", err)
}
networkClassAdapter := dispatcheradapter.NewNetworkClassAdapter(privatev1.NewNetworkClassesClient(grpcConn))
networkClassAdapter := dispatcheradapter.NewNetworkClassAdapter(networkClassesClient)
resolver = dispatcher.NewResolver(networkClassAdapter, disc)

if err := setupNetworkClassCapabilitiesController(
Expand All @@ -535,9 +539,11 @@ func setupNetworkingControllers(
); err != nil {
return err
}

if err := setupSubnetControllers(
mgr, localMgr, grpcConn, networkingNamespace,
networkingProvider, statusPollInterval, maxJobHistory, targetCluster, resolver,
networkClassesClient,
); err != nil {
return err
}
Expand Down Expand Up @@ -630,6 +636,7 @@ func setupSubnetControllers(
networkingNamespace string, provider provisioning.ProvisioningProvider,
statusPollInterval time.Duration, maxJobHistory int, targetCluster multicluster.ClusterName,
resolver *dispatcher.Resolver,
networkClassesClient privatev1.NetworkClassesClient,
) error {
if grpcConn != nil {
if err := controller.NewSubnetFeedbackReconciler(
Expand All @@ -640,6 +647,7 @@ func setupSubnetControllers(
}
if err := controller.NewSubnetReconciler(
mgr, networkingNamespace, provider, statusPollInterval, maxJobHistory, targetCluster, resolver,
networkClassesClient,
).SetupWithManager(mgr); err != nil {
return fmt.Errorf("subnet controller: %w", err)
}
Expand Down
9 changes: 9 additions & 0 deletions osac-operator/config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@ rules:
- get
- list
- watch
- apiGroups:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] permission-expansion

The RBAC rule for metallb.io/ipaddresspools includes the patch verb, which is not used by any code path in this diff. Removing unused verbs is a least-privilege improvement.

Suggested fix: Remove the patch verb from the metallb.io/ipaddresspools rule.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] permission-expansion

The manager-role ClusterRole gains create/delete/get/update on metallb.io/ipaddresspools. RBAC correctly omits list/watch/patch (least-privilege). Expansion tracked by OSAC-2350/OSAC-2482.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct — least-privilege RBAC with only the verbs the controller uses. Tracked by OSAC-2350.

- metallb.io
resources:
- ipaddresspools
verbs:
- create
- delete
- get
- update
- apiGroups:
- osac.openshift.io
resources:
Expand Down
29 changes: 29 additions & 0 deletions osac-operator/helpers/helpers_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
Copyright 2026.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package helpers

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestHelpers(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Helpers Suite")
}
79 changes: 79 additions & 0 deletions osac-operator/helpers/viprange.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
Copyright 2026.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package helpers

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] naming-convention

Missing Apache 2.0 copyright headers in viprange.go and viprange_test.go.


import (
"encoding/binary"
"fmt"
"net"
)

// ComputeVIPRange computes the VIP sub-range from a subnet CIDR and a VIP
// prefix length. The VIP range occupies the highest addresses of the subnet.
//

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] edge-case

For subnet CIDRs with very small prefix lengths (e.g., /0), uint32(1) << (32 - subnetOnes) overflows uint32.

// Example: ComputeVIPRange("10.0.1.0/24", 28) returns "10.0.1.240-10.0.1.255".
func ComputeVIPRange(subnetCIDR string, vipPrefixLength int) (start, end net.IP, err error) {
_, ipNet, err := net.ParseCIDR(subnetCIDR)
if err != nil {
return nil, nil, fmt.Errorf("invalid subnet CIDR %q: %w", subnetCIDR, err)
}

subnetOnes, subnetBits := ipNet.Mask.Size()
if subnetBits != 32 {
return nil, nil, fmt.Errorf("only IPv4 CIDRs are supported, got %q", subnetCIDR)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] error-message-format

Error message uses Go parameter name vipPrefixLength in camelCase. The codebase convention uses human-readable lowercase descriptions.

Suggested fix: Use VIP prefix length instead of vipPrefixLength.

}

if vipPrefixLength <= subnetOnes || vipPrefixLength > 32 {
return nil, nil, fmt.Errorf(
"VIP prefix length %d must be greater than subnet prefix %d and at most 32",
vipPrefixLength, subnetOnes,
)
}

subnetIP := binary.BigEndian.Uint32(ipNet.IP.To4())
subnetSize := uint32(1) << (32 - subnetOnes)
vipSize := uint32(1) << (32 - vipPrefixLength)

vipStart := subnetIP + subnetSize - vipSize
vipEnd := subnetIP + subnetSize - 1

startIP := make(net.IP, 4)
endIP := make(net.IP, 4)
binary.BigEndian.PutUint32(startIP, vipStart)
binary.BigEndian.PutUint32(endIP, vipEnd)

return startIP, endIP, nil
}

// FormatVIPRangeCIDR returns the VIP sub-range as a CIDR string suitable for
// MetalLB IPAddressPool addresses.
func FormatVIPRangeCIDR(subnetCIDR string, vipPrefixLength int) (string, error) {
start, _, err := ComputeVIPRange(subnetCIDR, vipPrefixLength)
if err != nil {
return "", err
}
return fmt.Sprintf("%s/%d", start.String(), vipPrefixLength), nil
}

// FormatVIPRangeDash returns the VIP sub-range as a "start-end" string.
func FormatVIPRangeDash(subnetCIDR string, vipPrefixLength int) (string, error) {
start, end, err := ComputeVIPRange(subnetCIDR, vipPrefixLength)
if err != nil {
return "", err
}
return fmt.Sprintf("%s-%s", start.String(), end.String()), nil
}
83 changes: 83 additions & 0 deletions osac-operator/helpers/viprange_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
Copyright 2026.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package helpers

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("ComputeVIPRange", func() {
type testCase struct {
subnetCIDR string
vipPrefixLength int
wantStart string
wantEnd string
}

DescribeTable("computes the correct VIP sub-range",
func(tc testCase) {
start, end, err := ComputeVIPRange(tc.subnetCIDR, tc.vipPrefixLength)
Expect(err).NotTo(HaveOccurred())
Expect(start.String()).To(Equal(tc.wantStart))
Expect(end.String()).To(Equal(tc.wantEnd))
},
Entry("/24 subnet with /28 VIP prefix", testCase{
subnetCIDR: "10.0.1.0/24", vipPrefixLength: 28,
wantStart: "10.0.1.240", wantEnd: "10.0.1.255",
}),
Entry("/24 subnet with /26 VIP prefix", testCase{
subnetCIDR: "10.0.1.0/24", vipPrefixLength: 26,
wantStart: "10.0.1.192", wantEnd: "10.0.1.255",
}),
Entry("/22 subnet with /28 VIP prefix", testCase{
subnetCIDR: "172.16.0.0/22", vipPrefixLength: 28,
wantStart: "172.16.3.240", wantEnd: "172.16.3.255",
}),
Entry("/24 subnet with /30 VIP prefix", testCase{
subnetCIDR: "192.168.1.0/24", vipPrefixLength: 30,
wantStart: "192.168.1.252", wantEnd: "192.168.1.255",
}),
)

DescribeTable("rejects invalid inputs",
func(subnetCIDR string, vipPrefixLength int) {
_, _, err := ComputeVIPRange(subnetCIDR, vipPrefixLength)
Expect(err).To(HaveOccurred())
},
Entry("invalid CIDR", "not-a-cidr", 28),
Entry("VIP prefix equal to subnet prefix", "10.0.1.0/24", 24),
Entry("VIP prefix smaller than subnet prefix", "10.0.1.0/24", 20),
Entry("VIP prefix exceeds 32", "10.0.1.0/24", 33),
)
})

var _ = Describe("FormatVIPRangeCIDR", func() {
It("formats the VIP range as a CIDR string", func() {
cidr, err := FormatVIPRangeCIDR("10.0.1.0/24", 28)
Expect(err).NotTo(HaveOccurred())
Expect(cidr).To(Equal("10.0.1.240/28"))
})
})

var _ = Describe("FormatVIPRangeDash", func() {
It("formats the VIP range as a start-end string", func() {
dash, err := FormatVIPRangeDash("10.0.1.0/24", 28)
Expect(err).NotTo(HaveOccurred())
Expect(dash).To(Equal("10.0.1.240-10.0.1.255"))
})
})
Loading
Loading