Skip to content

Conversation

@camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Nov 4, 2025

This PR fully ports the code over CRD upgrade safety from the release-4.20 branch.

We’ve also embedded the third-party part here.

To verify this, you can run:

Run: git diff upstream/release-4.20 -- internal/operator-controller/rukpak/preflights/crdupgradesafety
See that just imported are changed:

diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go
index efa86a191..d317d1f13 100644
--- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go
+++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go
@@ -12,23 +12,24 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"sigs.k8s.io/crdify/pkg/config"
-	"sigs.k8s.io/crdify/pkg/runner"
-	"sigs.k8s.io/crdify/pkg/validations"
-	"sigs.k8s.io/crdify/pkg/validations/property"
+
+	crdifyconfig "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/config"
+	crdifyrunner "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/runner"
+	crdifyvalidations "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/validations"
+	crdifyproperty "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/validations/property"
 
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
 )
 
 type Option func(p *Preflight)
 
-func WithConfig(cfg *config.Config) Option {
+func WithConfig(cfg *crdifyconfig.Config) Option {
 	return func(p *Preflight) {
 		p.config = cfg
 	}
 }
 
-func WithRegistry(reg validations.Registry) Option {
+func WithRegistry(reg crdifyvalidations.Registry) Option {
 	return func(p *Preflight) {
 		p.registry = reg
 	}
@@ -36,8 +37,8 @@ func WithRegistry(reg validations.Registry) Option {
 
 type Preflight struct {
 	crdClient apiextensionsv1client.CustomResourceDefinitionInterface
-	config    *config.Config
-	registry  validations.Registry
+	config    *crdifyconfig.Config
+	registry  crdifyvalidations.Registry
 }
 
 func NewPreflight(crdCli apiextensionsv1client.CustomResourceDefinitionInterface, opts ...Option) *Preflight {
@@ -72,7 +73,7 @@ func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release) erro
 		return fmt.Errorf("parsing release %q objects: %w", rel.Name, err)
 	}
 
-	runner, err := runner.New(p.config, p.registry)
+	runner, err := crdifyrunner.New(p.config, p.registry)
 	if err != nil {
 		return fmt.Errorf("creating CRD validation runner: %w", err)
 	}
@@ -117,37 +118,37 @@ func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release) erro
 	return errors.Join(validateErrors...)
 }
 
-func defaultConfig() *config.Config {
-	return &config.Config{
+func defaultConfig() *crdifyconfig.Config {
+	return &crdifyconfig.Config{
 		// Ignore served version validations if conversion policy is set.
-		Conversion: config.ConversionPolicyIgnore,
+		Conversion: crdifyconfig.ConversionPolicyIgnore,
 		// Fail-closed by default
-		UnhandledEnforcement: config.EnforcementPolicyError,
+		UnhandledEnforcement: crdifyconfig.EnforcementPolicyError,
 		// Use the default validation configurations as they are
 		// the strictest possible.
-		Validations: []config.ValidationConfig{
+		Validations: []crdifyconfig.ValidationConfig{
 			// Do not enforce the description validation
 			// because OLM should not block on field description changes.
 			{
 				Name:        "description",
-				Enforcement: config.EnforcementPolicyNone,
+				Enforcement: crdifyconfig.EnforcementPolicyNone,
 			},
 			{
 				Name:        "enum",
-				Enforcement: config.EnforcementPolicyError,
+				Enforcement: crdifyconfig.EnforcementPolicyError,
 				Configuration: map[string]interface{}{
-					"additionPolicy": property.AdditionPolicyAllow,
+					"additionPolicy": crdifyproperty.AdditionPolicyAllow,
 				},
 			},
 		},
 	}
 }
 
-func defaultRegistry() validations.Registry {
-	return runner.DefaultRegistry()
+func defaultRegistry() crdifyvalidations.Registry {
+	return crdifyrunner.DefaultRegistry()
 }
 
-func crdWideErrors(results *runner.Results) []error {
+func crdWideErrors(results *crdifyrunner.Results) []error {
 	if results == nil {
 		return nil
 	}
@@ -162,7 +163,7 @@ func crdWideErrors(results *runner.Results) []error {
 	return errs
 }
 
-func sameVersionErrors(results *runner.Results) []error {
+func sameVersionErrors(results *crdifyrunner.Results) []error {
 	if results == nil {
 		return nil
 	}
@@ -185,7 +186,7 @@ func sameVersionErrors(results *runner.Results) []error {
 	return errs
 }
 
-func servedVersionErrors(results *runner.Results) []error {
+func servedVersionErrors(results *crdifyrunner.Results) []error {
 	if results == nil {
 		return nil
 	}
diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go
index 2d2a2065f..4ed62e9e0 100644
--- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go
+++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go
@@ -15,7 +15,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	crdifyconfig "sigs.k8s.io/crdify/pkg/config"
+
+	crdifyconfig "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/config"
 
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"

AND:

Run git diff upstream/release-4.20 -- internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests to ensure that all testdata used on the tests is either ported.

@openshift-ci openshift-ci bot requested review from bentito and joelanford November 4, 2025 17:17
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 4, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: camilamacedo86
Once this PR has been reviewed and has the lgtm label, please assign oceanc80 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@camilamacedo86 camilamacedo86 changed the title UPSTREAM: <carry>: Port code from crdify Port CRD upgrade safety carry from 4.20 without bumping deps Nov 4, 2025
@camilamacedo86 camilamacedo86 changed the title Port CRD upgrade safety carry from 4.20 without bumping deps OPRUN-4216, OCPBUGS-63405: Port CRD upgrade safety carry from 4.20 without bumping deps Nov 4, 2025
@openshift-ci-robot openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Nov 4, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 4, 2025

@camilamacedo86: This pull request references OPRUN-4216 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.z" version, but no target version was set.

This pull request references Jira Issue OCPBUGS-63405, which is valid. The bug has been moved to the POST state.

7 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.19.z) matches configured target version for branch (4.19.z)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)
  • release note type set to "Release Note Not Required"
  • dependent bug Jira Issue OCPBUGS-61890 is in the state Closed (Done), which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA))
  • dependent Jira Issue OCPBUGS-61890 targets the "4.20.z" version, which is one of the valid target versions: 4.20.0, 4.20.z
  • bug has dependents

Requesting review from QA contact:
/cc @Xia-Zhao-rh

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

This PR fully ports the code over CRD upgrade safety from the release-4.20 branch.

We’ve also embedded the third-party part here.

To verify this, you can run:

Run: git diff upstream/release-4.20 -- internal/operator-controller/rukpak/preflights/crdupgradesafety
See that just imported are changed:

diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go
index efa86a191..d317d1f13 100644
--- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go
+++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go
@@ -12,23 +12,24 @@ import (
	apierrors "k8s.io/apimachinery/pkg/api/errors"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/apimachinery/pkg/runtime"
-	"sigs.k8s.io/crdify/pkg/config"
-	"sigs.k8s.io/crdify/pkg/runner"
-	"sigs.k8s.io/crdify/pkg/validations"
-	"sigs.k8s.io/crdify/pkg/validations/property"
+
+	crdifyconfig "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/config"
+	crdifyrunner "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/runner"
+	crdifyvalidations "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/validations"
+	crdifyproperty "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/validations/property"

	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
)

type Option func(p *Preflight)

-func WithConfig(cfg *config.Config) Option {
+func WithConfig(cfg *crdifyconfig.Config) Option {
	return func(p *Preflight) {
		p.config = cfg
	}
}

-func WithRegistry(reg validations.Registry) Option {
+func WithRegistry(reg crdifyvalidations.Registry) Option {
	return func(p *Preflight) {
		p.registry = reg
	}
@@ -36,8 +37,8 @@ func WithRegistry(reg validations.Registry) Option {

type Preflight struct {
	crdClient apiextensionsv1client.CustomResourceDefinitionInterface
-	config    *config.Config
-	registry  validations.Registry
+	config    *crdifyconfig.Config
+	registry  crdifyvalidations.Registry
}

func NewPreflight(crdCli apiextensionsv1client.CustomResourceDefinitionInterface, opts ...Option) *Preflight {
@@ -72,7 +73,7 @@ func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release) erro
		return fmt.Errorf("parsing release %q objects: %w", rel.Name, err)
	}

-	runner, err := runner.New(p.config, p.registry)
+	runner, err := crdifyrunner.New(p.config, p.registry)
	if err != nil {
		return fmt.Errorf("creating CRD validation runner: %w", err)
	}
@@ -117,37 +118,37 @@ func (p *Preflight) runPreflight(ctx context.Context, rel *release.Release) erro
	return errors.Join(validateErrors...)
}

-func defaultConfig() *config.Config {
-	return &config.Config{
+func defaultConfig() *crdifyconfig.Config {
+	return &crdifyconfig.Config{
		// Ignore served version validations if conversion policy is set.
-		Conversion: config.ConversionPolicyIgnore,
+		Conversion: crdifyconfig.ConversionPolicyIgnore,
		// Fail-closed by default
-		UnhandledEnforcement: config.EnforcementPolicyError,
+		UnhandledEnforcement: crdifyconfig.EnforcementPolicyError,
		// Use the default validation configurations as they are
		// the strictest possible.
-		Validations: []config.ValidationConfig{
+		Validations: []crdifyconfig.ValidationConfig{
			// Do not enforce the description validation
			// because OLM should not block on field description changes.
			{
				Name:        "description",
-				Enforcement: config.EnforcementPolicyNone,
+				Enforcement: crdifyconfig.EnforcementPolicyNone,
			},
			{
				Name:        "enum",
-				Enforcement: config.EnforcementPolicyError,
+				Enforcement: crdifyconfig.EnforcementPolicyError,
				Configuration: map[string]interface{}{
-					"additionPolicy": property.AdditionPolicyAllow,
+					"additionPolicy": crdifyproperty.AdditionPolicyAllow,
				},
			},
		},
	}
}

-func defaultRegistry() validations.Registry {
-	return runner.DefaultRegistry()
+func defaultRegistry() crdifyvalidations.Registry {
+	return crdifyrunner.DefaultRegistry()
}

-func crdWideErrors(results *runner.Results) []error {
+func crdWideErrors(results *crdifyrunner.Results) []error {
	if results == nil {
		return nil
	}
@@ -162,7 +163,7 @@ func crdWideErrors(results *runner.Results) []error {
	return errs
}

-func sameVersionErrors(results *runner.Results) []error {
+func sameVersionErrors(results *crdifyrunner.Results) []error {
	if results == nil {
		return nil
	}
@@ -185,7 +186,7 @@ func sameVersionErrors(results *runner.Results) []error {
	return errs
}

-func servedVersionErrors(results *runner.Results) []error {
+func servedVersionErrors(results *crdifyrunner.Results) []error {
	if results == nil {
		return nil
	}
diff --git a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go
index 2d2a2065f..4ed62e9e0 100644
--- a/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go
+++ b/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.go
@@ -15,7 +15,8 @@ import (
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/apimachinery/pkg/runtime"
	"k8s.io/apimachinery/pkg/runtime/schema"
-	crdifyconfig "sigs.k8s.io/crdify/pkg/config"
+
+	crdifyconfig "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/config"

	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"

AND:

Run git diff upstream/release-4.20 -- internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests to ensure that all testdata used on the tests is either ported.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested a review from Xia-Zhao-rh November 4, 2025 17:23
crdifyconfig "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/config"
crdifyrunner "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/runner"
crdifyvalidations "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/validations"
crdifyproperty "github.com/operator-framework/operator-controller/internal/thirdparty/crdify/pkg/validations/property"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tmshort

We have no bumper out of main, so I think we can safely embeed here
If you have a better path thought please let me know.

@camilamacedo86
Copy link
Contributor Author

/test openshift-e2e-aws

@jianzhangbjz
Copy link
Contributor

/assign @Xia-Zhao-rh

@tmshort
Copy link
Contributor

tmshort commented Nov 7, 2025

I did a comparison against main, and the copies and renaming (for the new location) seem fine. The only thing that's missing in internal/thirdparty/crdify is the LICENSE file (which is Apache), and possibly a README (?) indicating where this code originally came from?

@camilamacedo86
Copy link
Contributor Author

Hi @tmshort

I did a comparison against main, and the copies and renaming (for the new location) seem fine. The only thing that's missing in internal/thirdparty/crdify is the LICENSE file (which is Apache), and possibly a README (?) indicating where this code originally came from?

That is done. Thank you :-)

@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Nov 8, 2025

Hi @Xia-Zhao-rh 👋

Could you please validate this one?
Also, can you check all the related OCPBugs we have?

If any of them are still unresolved, it means they’re also not fixed in 4.20.
In that case, we’ll need to create a specific bug to track and port all fixes, including upgrading sigs.k8s.io/crdify to version v0.5.0 (as used in 4.21).

Once this is merged in 4.20, we can bring the same changes to 4.19.
We must ensure that 4.19 and 4.20 remain identical at this point.

PS.: I think the above will be required.
@jianzhangbjz, here #527 (comment), it seems that the validation only one bug (not all of them).
My 2 bet cents is that OCPBUGS-63405 is fixed for 4.20 but not the others bugs
That means we will need to address those in a follow-up by ensuring that 4.20 is equal to 4.21 and using the latest version of crdiff, and then porting it here.

Thanks!

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 8, 2025

@camilamacedo86: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws-ovn 0bcc4f0 link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants