Skip to content
Open
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
49 changes: 42 additions & 7 deletions cmd/branchingconfigmanagers/tide-config-manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,25 +484,60 @@ func (ve *verifiedEvent) ModifyQuery(q *prowconfig.TideQuery, repo string) {
q.Labels = sets.List(reqLabels)
}

// isVersionedBranch checks if a branch name matches release-4.x or openshift-4.x pattern
// isVersionedBranch checks if a branch name matches release-X.y or openshift-X.y pattern for modern versions (4.x+)
func isVersionedBranch(branch string) bool {
if strings.HasPrefix(branch, "release-4.") {
versionPart := strings.TrimPrefix(branch, "release-4.")
if isValidMinorVersion(versionPart) {
// Check for release-X.y pattern (supporting any major version >= 4)
if strings.HasPrefix(branch, "release-") {
versionPart := strings.TrimPrefix(branch, "release-")
if isValidVersionString(versionPart) {
return true
}
}

if strings.HasPrefix(branch, "openshift-4.") {
versionPart := strings.TrimPrefix(branch, "openshift-4.")
if isValidMinorVersion(versionPart) {
// Check for openshift-X.y pattern (supporting any major version >= 4)
if strings.HasPrefix(branch, "openshift-") {
versionPart := strings.TrimPrefix(branch, "openshift-")
if isValidVersionString(versionPart) {
return true
}
}

return false
}

// isValidVersionString checks if a version string is in X.y format where X >= 4
func isValidVersionString(version string) bool {
parts := strings.Split(version, ".")
if len(parts) != 2 {
return false
}

// Check if major version is a number >= 4
majorStr, minorStr := parts[0], parts[1]

// Validate major version
if len(majorStr) == 0 {
return false
}
for _, char := range majorStr {
if char < '0' || char > '9' {
return false
}
}

// Convert to int and check if >= 4
major := 0
for _, char := range majorStr {
major = major*10 + int(char-'0')
}
if major < 4 {
return false
}

// Validate minor version using existing function
return isValidMinorVersion(minorStr)
}

// isValidMinorVersion checks if a string represents a valid minor version (e.g., "9", "10", "15")
func isValidMinorVersion(version string) bool {
if version == "" {
Expand Down
227 changes: 227 additions & 0 deletions cmd/branchingconfigmanagers/tide-config-manager/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,36 @@ func TestIsVersionedBranch(t *testing.T) {
branch: "openshift-4.15",
expected: true,
},
{
name: "valid release-5.x branch",
branch: "release-5.1",
expected: true,
},
{
name: "valid openshift-5.x branch",
branch: "openshift-5.2",
expected: true,
},
{
name: "valid release-5.x branch with double digit minor",
branch: "release-5.10",
expected: true,
},
{
name: "valid openshift-5.x branch with double digit minor",
branch: "openshift-5.15",
expected: true,
},
{
name: "invalid release-5.x branch with version 0",
branch: "release-5.0",
expected: false,
},
{
name: "invalid openshift-5.x branch with version 0",
branch: "openshift-5.0",
expected: false,
},
{
name: "invalid release branch with version 0",
branch: "release-4.0",
Expand Down Expand Up @@ -674,6 +704,56 @@ func TestIsVersionedBranch(t *testing.T) {
branch: "openshift-4.",
expected: false,
},
{
name: "invalid release-5.x branch with non-numeric version",
branch: "release-5.abc",
expected: false,
},
{
name: "invalid openshift-5.x branch with non-numeric version",
branch: "openshift-5.x",
expected: false,
},
{
name: "release-5.x branch without version",
branch: "release-5.",
expected: false,
},
{
name: "openshift-5.x branch without version",
branch: "openshift-5.",
expected: false,
},
{
name: "valid release-6.x branch (future-proofing)",
branch: "release-6.1",
expected: true,
},
{
name: "valid openshift-6.x branch (future-proofing)",
branch: "openshift-6.2",
expected: true,
},
{
name: "valid release-10.x branch (future-proofing)",
branch: "release-10.5",
expected: true,
},
{
name: "valid openshift-15.x branch (future-proofing)",
branch: "openshift-15.20",
expected: true,
},
{
name: "invalid release-3.x branch (too old)",
branch: "release-3.11",
expected: false,
},
{
name: "invalid openshift-3.x branch (too old)",
branch: "openshift-3.11",
expected: false,
},
}

for _, tt := range tests {
Expand All @@ -685,3 +765,150 @@ func TestIsVersionedBranch(t *testing.T) {
})
}
}

func TestNewPreGeneralAvailability(t *testing.T) {
tests := []struct {
name string
current string
expected []string
}{
{
name: "4.9 version",
current: "4.9",
expected: []string{"release-4.9", "openshift-4.9"},
},
{
name: "5.1 version",
current: "5.1",
expected: []string{"release-5.1", "openshift-5.1"},
},
{
name: "4.15 version",
current: "4.15",
expected: []string{"release-4.15", "openshift-4.15"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
delegate := newSharedDataDelegate()
event := newPreGeneralAvailability(tt.current, delegate)

// Check that the event contains the expected branches
for _, expectedBranch := range tt.expected {
if !event.openshiftReleaseBranches.Has(expectedBranch) {
t.Errorf("Expected branch %s not found in openshiftReleaseBranches", expectedBranch)
}
}

// Check that delegate is set
if event.sharedDataDelegate == nil {
t.Error("sharedDataDelegate should not be nil")
}
})
}
}

func TestPreGeneralAvailabilityEventModifyQuery(t *testing.T) {
delegate := newSharedDataDelegate()
event := newPreGeneralAvailability("4.9", delegate)

tests := []struct {
name string
inputLabels []string
inputBranches []string
expectedLabels []string
repo string
}{
{
name: "adds staff-eng-approved when backport-risk-assessed present for release branch",
inputLabels: []string{backportRiskAssessed},
inputBranches: []string{"release-4.9"},
expectedLabels: []string{backportRiskAssessed, staffEngApproved},
repo: "openshift/dummy",
},
{
name: "adds staff-eng-approved when backport-risk-assessed present for openshift branch",
inputLabels: []string{backportRiskAssessed},
inputBranches: []string{"openshift-4.9"},
expectedLabels: []string{backportRiskAssessed, staffEngApproved},
repo: "openshift/dummy",
},
{
name: "adds staff-eng-approved for multiple matching branches",
inputLabels: []string{backportRiskAssessed, "other-label"},
inputBranches: []string{"release-4.9", "openshift-4.9"},
expectedLabels: []string{backportRiskAssessed, "other-label", staffEngApproved},
repo: "openshift/dummy",
},
{
name: "no change when backport-risk-assessed not present",
inputLabels: []string{"other-label"},
inputBranches: []string{"release-4.9"},
expectedLabels: []string{"other-label"},
repo: "openshift/dummy",
},
{
name: "no change for non-matching branches",
inputLabels: []string{backportRiskAssessed},
inputBranches: []string{"release-4.8", "main"},
expectedLabels: []string{backportRiskAssessed},
repo: "openshift/dummy",
},
{
name: "no change when staff-eng-approved already present",
inputLabels: []string{backportRiskAssessed, staffEngApproved},
inputBranches: []string{"release-4.9"},
expectedLabels: []string{backportRiskAssessed, staffEngApproved},
repo: "openshift/dummy",
},
{
name: "handles empty branches",
inputLabels: []string{backportRiskAssessed},
inputBranches: []string{},
expectedLabels: []string{backportRiskAssessed},
repo: "openshift/dummy",
},
{
name: "handles mixed branches with some matching",
inputLabels: []string{backportRiskAssessed},
inputBranches: []string{"main", "release-4.9", "release-4.8"},
expectedLabels: []string{backportRiskAssessed, staffEngApproved},
repo: "openshift/dummy",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
query := &prowconfig.TideQuery{
Labels: tt.inputLabels,
IncludedBranches: tt.inputBranches,
}

event.ModifyQuery(query, tt.repo)

actualLabels := sets.New[string](query.Labels...)
expectedLabels := sets.New[string](tt.expectedLabels...)

if !actualLabels.Equal(expectedLabels) {
t.Errorf("Expected labels: %v, got: %v", sets.List(expectedLabels), sets.List(actualLabels))
}
})
}
}

func TestPreGeneralAvailabilityEventGetDataFromProwConfig(t *testing.T) {
delegate := newSharedDataDelegate()
event := newPreGeneralAvailability("4.9", delegate)

// This method is currently empty, but we test that it doesn't panic
t.Run("does not panic", func(t *testing.T) {
defer func() {
if r := recover(); r != nil {
t.Errorf("GetDataFromProwConfig panicked: %v", r)
}
}()

event.GetDataFromProwConfig(&prowconfig.ProwConfig{})
})
}
62 changes: 50 additions & 12 deletions pkg/api/leases.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package api

import (
"fmt"
"strconv"
"strings"
)
Expand Down Expand Up @@ -42,28 +43,65 @@ func IPPoolLeaseForTest(s *MultiStageTestConfigurationLiteral, metadata Metadata
}

const (
openshiftBranch = "openshift-4."
releaseBranch = "release-4."
minimumVersion = 16
minimumMajorVersion = 4
minimumMinorVersion = 16
)

// Currently, we only have the ability to utilize IP pools in 4.16 and later, we want to make sure not to allocate them
// on earlier versions
// on earlier versions. 5.x and later versions are supported.
func branchValidForIPPoolLease(branch string) bool {
if branch == "master" || branch == "main" {
return true
}
var version string
if strings.HasPrefix(branch, openshiftBranch) {
version = strings.TrimPrefix(branch, openshiftBranch)

// Check for openshift-X.Y or release-X.Y pattern
var majorVersion, minorVersion int
var err error

if strings.HasPrefix(branch, "openshift-") {
versionPart := strings.TrimPrefix(branch, "openshift-")
if majorVersion, minorVersion, err = parseMajorMinorVersion(versionPart); err != nil {
return false
}
} else if strings.HasPrefix(branch, "release-") {
versionPart := strings.TrimPrefix(branch, "release-")
if majorVersion, minorVersion, err = parseMajorMinorVersion(versionPart); err != nil {
return false
}
} else {
return false
}

// 5.x and later are supported
if majorVersion >= 5 {
return true
}

// For 4.x, check minimum minor version
if majorVersion == 4 {
return minorVersion >= minimumMinorVersion
}
if strings.HasPrefix(branch, releaseBranch) {
version = strings.TrimPrefix(branch, releaseBranch)

// Earlier major versions not supported
return false
}

// parseMajorMinorVersion parses a version string like "4.16" or "5.1" into major and minor components
func parseMajorMinorVersion(version string) (int, int, error) {
parts := strings.Split(version, ".")
if len(parts) != 2 {
return 0, 0, fmt.Errorf("invalid version format: %s", version)
}
number, err := strconv.Atoi(version)

major, err := strconv.Atoi(parts[0])
if err != nil {
return false
return 0, 0, fmt.Errorf("invalid major version: %s", parts[0])
}

minor, err := strconv.Atoi(parts[1])
if err != nil {
return 0, 0, fmt.Errorf("invalid minor version: %s", parts[1])
}

return number >= minimumVersion
return major, minor, nil
}
Loading