Skip to content

OTA-1546: Reconcile cv.spec.desiredUpdate.acceptRisks #1284

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

Open
hongkailiu wants to merge 3 commits into
base: main
Choose a base branch
from
Open

OTA-1546: Reconcile cv.spec.desiredUpdate.acceptRisks #1284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
83c1a18
to-be-replaced by api#2360
hongkailiu Jan 2, 2026
8b12479
bump o/client-go
hongkailiu Jan 2, 2026
32fa03f
OTA-1546: Reconcile cv.spec.desiredUpdate.acceptRisks
hongkailiu Jan 2, 2026
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
6 changes: 5 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/onsi/ginkgo/v2 v2.21.0
github.com/onsi/gomega v1.35.1
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45
github.com/openshift/api v0.0.0-20251127005036-0e3c378fdedc
github.com/openshift/api v0.0.0-20251222154221-d4b2fef98af2
github.com/openshift/client-go v0.0.0-20251201171210-333716c1124a
github.com/openshift/library-go v0.0.0-20251120164824-14a789e09884
github.com/operator-framework/api v0.17.1
Expand Down Expand Up @@ -93,3 +93,7 @@ require (
)

replace github.com/onsi/ginkgo/v2 => github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20241205171354-8006f302fd12

replace github.com/openshift/api => github.com/hongkailiu/api v0.0.0-20251223025917-1321bde686ff
Copy link

@coderabbitai coderabbitai bot Jan 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n go.mod | sed -n '1,110p'

Repository: openshift/cluster-version-operator

Length of output: 5226

🏁 Script executed:

# Search for comments related to hongkailiu or acceptRisks (mentioned in original comment)
rg -i "hongkailiu|acceptRisks|accept.risk" --type go --type md -B 2 -A 2

Repository: openshift/cluster-version-operator

Length of output: 8135

🏁 Script executed:

# Check what types from openshift/api are being imported
rg "from github.com/openshift/api|import.*openshift/api" --type go -A 3 | head -40

Repository: openshift/cluster-version-operator

Length of output: 60

🏁 Script executed:

# Search for actual imports from openshift/api package
rg "configv1|appv1|imagev1" --type go | head -20

Repository: openshift/cluster-version-operator

Length of output: 2314

🏁 Script executed:

# Check git log for recent changes to go.mod related to openshift/api
git log --oneline -20 go.mod | head -20

Repository: openshift/cluster-version-operator

Length of output: 140

🏁 Script executed:

# Check the commit details for go.mod changes
git show cd8eab20 -- go.mod

Repository: openshift/cluster-version-operator

Length of output: 4955

🏁 Script executed:

# Check what changed in the commit - look at actual code changes (not just go.mod)
git show cd8eab20 --stat | head -30

Repository: openshift/cluster-version-operator

Length of output: 1769

🏁 Script executed:

# Check the full commit message and any PR details
git show cd8eab20 --format=fuller | head -50

Repository: openshift/cluster-version-operator

Length of output: 2089

🏁 Script executed:

# Check if there are any comments or documentation about the fork in the repo
rg -i "fork|hongkailiu" --type md -B 2 -A 2

Repository: openshift/cluster-version-operator

Length of output: 858

🏁 Script executed:

# Check if there's any PR description or related documentation about why the fork is needed
ls -la | grep -E "PULL_REQUEST|PR_|ISSUE"

Repository: openshift/cluster-version-operator

Length of output: 60

🏁 Script executed:

# Check the most recent changes to understand if this is a new repository initialization
git log --oneline --all | tail -10

Repository: openshift/cluster-version-operator

Length of output: 532

🏁 Script executed:

# Check if the fork repository exists and is accessible - search for any references to what changed in hongkailiu/api
rg "github.com/hongkailiu" . --type go --type md -B 1 -A 1

Repository: openshift/cluster-version-operator

Length of output: 60

🏁 Script executed:

# Check what specific types/interfaces are being used from openshift/api related to acceptRisks
rg "ConditionalUpdateRisk|AcceptRisks|ConditionalUpdate" --type go -B 1 -A 1 | head -40

Repository: openshift/cluster-version-operator

Length of output: 2535

🏁 Script executed:

# Check the exact types from configv1 that are used in the code
rg "desiredUpdate.AcceptRisks|spec.DesiredUpdate.AcceptRisks" --type go -B 2 -A 2

Repository: openshift/cluster-version-operator

Length of output: 60

🏁 Script executed:

# Check what types are imported and used from openshift/api
grep -r "DesiredUpdate\|ConditionalUpdateRisk\|AcceptRisks" pkg/ --include="*.go" | head -20

Repository: openshift/cluster-version-operator

Length of output: 1955

🏁 Script executed:

# Check what specific fields from configv1.Update are being accessed (particularly AcceptRisks)
rg "\.AcceptRisks|configv1\.Update" --type go -B 2 -A 2 | head -50

Repository: openshift/cluster-version-operator

Length of output: 3656

Replace directive must be removed or documented before merge—uses personal fork instead of upstream dependency.

The replace directive at line 97 redirects github.com/openshift/api to a personal fork (hongkailiu/api). This creates supply chain and availability risks:

  • Personal forks can be deleted, abandoned, or compromised, breaking builds and potentially introducing malicious changes.
  • CI/CD systems with restricted network access (air-gapped builds, corporate firewalls) cannot pull from personal repositories.
  • Unsustainable for production dependencies.

The code actively uses configv1 types from openshift/api (e.g., ConditionalUpdateRisk, Update, AcceptRisks fields). Before this PR merges:

  1. If temporary (development only): Remove the replace directive before merge.
  2. If required: Contribute missing features/types to canonical openshift/api and update this dependency to use the upstream version. Document the blocker (e.g., GitHub issue reference).
  3. If transitive compatibility fix: Add a comment in go.mod explaining the reasoning.
🤖 Prompt for AI Agents 
In go.mod around line 97, the replace directive redirects
github.com/openshift/api to a personal fork (hongkailiu/api) which must not be
left in the merge; either remove the replace before merging if it was only for
local/dev testing, or if the forked changes are required: open a public issue/PR
against the canonical github.com/openshift/api to get the needed types merged,
update the go.mod to require the upstream module version that contains those
changes (and remove the personal replace), or if this is an unavoidable
temporary transitive compatibility workaround, add a clear comment immediately
above the replace in go.mod documenting why the fork is used, link to the
blocker issue/PR, and include a TODO to remove it once upstream is updated so CI
and supply-chain concerns are visible.


replace github.com/openshift/client-go => github.com/openshift/client-go v0.0.0-20251223102348-558b0eef16bc
8 changes: 4 additions & 4 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db h1:097atOisP2aRj7vFgY
github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db/go.mod h1:vavhavw2zAxS5dIdcRluK6cSGGPlZynqzFM8NdvU144=
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/hongkailiu/api v0.0.0-20251223025917-1321bde686ff h1:D9VFJdZ/Kop93dbmdDJO4fal4vFzKMDtzrmiOc9J5oQ=
github.com/hongkailiu/api v0.0.0-20251223025917-1321bde686ff/go.mod h1:d5uzF0YN2nQQFA0jIEWzzOZ+edmo6wzlGLvx5Fhz4uY=
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
Expand Down Expand Up @@ -86,10 +88,8 @@ github.com/onsi/gomega v1.35.1 h1:Cwbd75ZBPxFSuZ6T+rN/WCb/gOc6YgFBXLlZLhC7Ds4=
github.com/onsi/gomega v1.35.1/go.mod h1:PvZbdDc8J6XJEpDK4HCuRBm8a6Fzp9/DmhC9C7yFlog=
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45 h1:hXpbYtP3iTh8oy/RKwKkcMziwchY3fIk95ciczf7cOA=
github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45/go.mod h1:6gkP5f2HL0meusT0Aim8icAspcD1cG055xxBZ9yC68M=
github.com/openshift/api v0.0.0-20251127005036-0e3c378fdedc h1:p83VYAk7mlqYZrMaKuu2bBNNLUKBUSCa6YzjPDqbOlk=
github.com/openshift/api v0.0.0-20251127005036-0e3c378fdedc/go.mod h1:d5uzF0YN2nQQFA0jIEWzzOZ+edmo6wzlGLvx5Fhz4uY=
github.com/openshift/client-go v0.0.0-20251201171210-333716c1124a h1:iJYjd+rxyjMa3Sk6Vg55secJ4yMrabr/ulnTiy+vDH0=
github.com/openshift/client-go v0.0.0-20251201171210-333716c1124a/go.mod h1:WD7m8ADeqiAKTHWx/mBoE/1MFMtnt9MYTyBOnf0L3LI=
github.com/openshift/client-go v0.0.0-20251223102348-558b0eef16bc h1:nIlRaJfr/yGjPV15MNF5eVHLAGyXFjcUzO+hXeWDDk8=
github.com/openshift/client-go v0.0.0-20251223102348-558b0eef16bc/go.mod h1:cs9BwTu96sm2vQvy7r9rOiltgu90M6ju2qIHFG9WU+o=
github.com/openshift/library-go v0.0.0-20251120164824-14a789e09884 h1:6512TMT14gnXQ4vyshzAQGjkctU0PO9G+y0tcBjw6Vk=
github.com/openshift/library-go v0.0.0-20251120164824-14a789e09884/go.mod h1:ErDfiIrPHH+menTP/B4LKd0nxFDdvCbTamAc6SWMIh8=
github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20241205171354-8006f302fd12 h1:AKx/w1qpS8We43bsRgf8Nll3CGlDHpr/WAXvuedTNZI=
Expand Down
102 changes: 76 additions & 26 deletions pkg/cvo/availableupdates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ import (
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog/v2"

configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/cluster-version-operator/lib/resourcemerge"
"github.com/openshift/cluster-version-operator/pkg/cincinnati"
"github.com/openshift/cluster-version-operator/pkg/clusterconditions"
"github.com/openshift/cluster-version-operator/pkg/internal"
)

const noArchitecture string = "NoArchitecture"
Expand Down Expand Up @@ -50,6 +52,12 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
channel := config.Spec.Channel
desiredArch := optr.getDesiredArchitecture(config.Spec.DesiredUpdate)
currentArch := optr.getCurrentArchitecture()
acceptRisks := sets.New[string]()
if config.Spec.DesiredUpdate != nil {
for _, risk := range config.Spec.DesiredUpdate.AcceptRisks {
acceptRisks.Insert(risk.Name)
}
}

// updates are only checked at most once per minimumUpdateCheckInterval or if the generation changes
optrAvailableUpdates := optr.getAvailableUpdates()
Expand Down Expand Up @@ -129,6 +137,9 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
optrAvailableUpdates.UpdateService = updateService
optrAvailableUpdates.Channel = channel
optrAvailableUpdates.Architecture = desiredArch
optrAvailableUpdates.ShouldReconcileAcceptRisks = optr.shouldReconcileAcceptRisks
optrAvailableUpdates.AcceptRisks = acceptRisks
optrAvailableUpdates.RiskConditions = map[string][]metav1.Condition{}
optrAvailableUpdates.ConditionRegistry = optr.conditionRegistry
optrAvailableUpdates.Condition = condition

Expand Down Expand Up @@ -167,9 +178,11 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
}

type availableUpdates struct {
UpdateService string
Channel string
Architecture string
UpdateService string
Channel string
Architecture string
ShouldReconcileAcceptRisks func() bool
AcceptRisks sets.Set[string]

// LastAttempt records the time of the most recent attempt at update
// retrieval, regardless of whether it was successful.
Expand All @@ -192,6 +205,11 @@ type availableUpdates struct {
ConditionRegistry clusterconditions.ConditionRegistry

Condition configv1.ClusterOperatorStatusCondition

// RiskConditions stores the condition for every risk (name, url, message, matchingRules).
// The key of the risk is represented by its name which is ensured by validating our graph-data.
// https://github.com/openshift/cincinnati-graph-data/blob/af701850c24b4a53426c2a5400c63895fdf9de60/hack/validate-blocked-edges.py#L25C77-L25C90
RiskConditions map[string][]metav1.Condition
}

func (u *availableUpdates) RecentlyAttempted(interval time.Duration) bool {
Expand Down Expand Up @@ -291,14 +309,17 @@ func (optr *Operator) getAvailableUpdates() *availableUpdates {
}

u := &availableUpdates{
UpdateService: optr.availableUpdates.UpdateService,
Channel: optr.availableUpdates.Channel,
Architecture: optr.availableUpdates.Architecture,
LastAttempt: optr.availableUpdates.LastAttempt,
LastSyncOrConfigChange: optr.availableUpdates.LastSyncOrConfigChange,
Current: *optr.availableUpdates.Current.DeepCopy(),
ConditionRegistry: optr.availableUpdates.ConditionRegistry, // intentionally not a copy, to preserve cache state
Condition: optr.availableUpdates.Condition,
UpdateService: optr.availableUpdates.UpdateService,
Channel: optr.availableUpdates.Channel,
Architecture: optr.availableUpdates.Architecture,
ShouldReconcileAcceptRisks: optr.shouldReconcileAcceptRisks,
AcceptRisks: optr.availableUpdates.AcceptRisks,
RiskConditions: optr.availableUpdates.RiskConditions,
LastAttempt: optr.availableUpdates.LastAttempt,
LastSyncOrConfigChange: optr.availableUpdates.LastSyncOrConfigChange,
Current: *optr.availableUpdates.Current.DeepCopy(),
ConditionRegistry: optr.availableUpdates.ConditionRegistry, // intentionally not a copy, to preserve cache state
Condition: optr.availableUpdates.Condition,
}

if optr.availableUpdates.Updates != nil {
Expand Down Expand Up @@ -427,7 +448,7 @@ func (u *availableUpdates) evaluateConditionalUpdates(ctx context.Context) {
return vi.GTE(vj)
})
for i, conditionalUpdate := range u.ConditionalUpdates {
condition := evaluateConditionalUpdate(ctx, conditionalUpdate.Risks, u.ConditionRegistry)
condition := evaluateConditionalUpdate(ctx, conditionalUpdate.Risks, u.ConditionRegistry, u.AcceptRisks, u.ShouldReconcileAcceptRisks, u.RiskConditions)

if condition.Status == metav1.ConditionTrue {
u.addUpdate(conditionalUpdate.Release)
Expand Down Expand Up @@ -478,13 +499,16 @@ func newRecommendedStatus(now, want metav1.ConditionStatus) metav1.ConditionStat
}

const (
recommendedReasonRisksNotExposed = "NotExposedToRisks"
recommendedReasonEvaluationFailed = "EvaluationFailed"
recommendedReasonMultiple = "MultipleReasons"
recommendedReasonRisksNotExposed = "NotExposedToRisks"
recommendedReasonExposedOnlyToAcceptedRisks = "ExposedOnlyToAcceptedRisks"
recommendedReasonEvaluationFailed = "EvaluationFailed"
recommendedReasonMultiple = "MultipleReasons"

// recommendedReasonExposed is used instead of the original name if it does
// not match the pattern for a valid k8s condition reason.
recommendedReasonExposed = "ExposedToRisks"

riskConditionReasonEvaluationFailed = "EvaluationFailed"
)

// Reasons follow same pattern as k8s Condition Reasons
Expand All @@ -493,18 +517,25 @@ var reasonPattern = regexp.MustCompile(`^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$

func newRecommendedReason(now, want string) string {
switch {
case now == recommendedReasonRisksNotExposed:
case now == recommendedReasonRisksNotExposed || now == recommendedReasonExposedOnlyToAcceptedRisks && now != want:
return want
case now == want:
case now == recommendedReasonExposedOnlyToAcceptedRisks && want == recommendedReasonRisksNotExposed || now == want:
return now
default:
return recommendedReasonMultiple
}
}

func evaluateConditionalUpdate(ctx context.Context, risks []configv1.ConditionalUpdateRisk, conditionRegistry clusterconditions.ConditionRegistry) metav1.Condition {
func evaluateConditionalUpdate(
ctx context.Context,
risks []configv1.ConditionalUpdateRisk,
conditionRegistry clusterconditions.ConditionRegistry,
acceptRisks sets.Set[string],
shouldReconcileAcceptRisks func() bool,
riskConditions map[string][]metav1.Condition,
) metav1.Condition {
recommended := metav1.Condition{
Type: ConditionalUpdateConditionTypeRecommended,
Type: internal.ConditionalUpdateConditionTypeRecommended,
Status: metav1.ConditionTrue,
// FIXME: ObservedGeneration? That would capture upstream/channel, but not necessarily the currently-reconciling version.
Reason: recommendedReasonRisksNotExposed,
Expand All @@ -513,18 +544,37 @@ func evaluateConditionalUpdate(ctx context.Context, risks []configv1.Conditional

var errorMessages []string
for _, risk := range risks {
riskCondition := metav1.Condition{
Type: internal.ConditionalUpdateRiskConditionTypeApplies,
Status: metav1.ConditionFalse,
}
if match, err := conditionRegistry.Match(ctx, risk.MatchingRules); err != nil {
msg := unknownExposureMessage(risk, err)
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionUnknown)
recommended.Reason = newRecommendedReason(recommended.Reason, recommendedReasonEvaluationFailed)
errorMessages = append(errorMessages, unknownExposureMessage(risk, err))
errorMessages = append(errorMessages, msg)
riskCondition.Status = metav1.ConditionUnknown
riskCondition.Reason = riskConditionReasonEvaluationFailed
riskCondition.Message = msg
} else if match {
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionFalse)
wantReason := recommendedReasonExposed
if reasonPattern.MatchString(risk.Name) {
wantReason = risk.Name
riskCondition.Status = metav1.ConditionTrue
if shouldReconcileAcceptRisks() && acceptRisks.Has(risk.Name) {
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionTrue)
recommended.Reason = newRecommendedReason(recommended.Reason, recommendedReasonExposedOnlyToAcceptedRisks)
recommended.Message = "The update is recommended, because either risk does not apply to this cluster or it is accepted by cluster admins."
klog.V(2).Infof("Risk with name %q is accepted by the cluster admin and thus not in the evaluation of conditional update", risk.Name)
} else {
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionFalse)
wantReason := recommendedReasonExposed
if reasonPattern.MatchString(risk.Name) {
wantReason = risk.Name
}
recommended.Reason = newRecommendedReason(recommended.Reason, wantReason)
errorMessages = append(errorMessages, fmt.Sprintf("%s %s", risk.Message, risk.URL))
}
recommended.Reason = newRecommendedReason(recommended.Reason, wantReason)
errorMessages = append(errorMessages, fmt.Sprintf("%s %s", risk.Message, risk.URL))
}
if _, ok := riskConditions[risk.Name]; !ok {
riskConditions[risk.Name] = []metav1.Condition{riskCondition}
}
}
if len(errorMessages) > 0 {
Expand Down
Loading