Skip to content

Conversation

@anik120
Copy link
Member

@anik120 anik120 commented Jan 20, 2026

Summary:

Implements Phase 1 of the variable configuration feature to achieve feature parity with
OLMv0's SubscriptionConfig for registry+v1 bundles.

RFC: https://docs.google.com/document/d/18O4qBvu5I4WIJgo5KU1opyUKcrfgk64xsI3tyXxmVEU/edit?tab=t.0#heading=h.x3tfh25grvnv

Details:

This PR adds infrastructure for JSON schema-based validation of registry+v1 bundle configuration
in ClusterExtension, including both watchNamespace and deploymentConfig fields:

  • Schema Generation Tool: Parses SubscriptionConfig using AST to extract field names/types

    • Fetches Kubernetes OpenAPI v3 specs from GitHub releases
    • Maps fields to official Kubernetes schemas
    • Generates schema with watchNamespace (for operator scope) and deploymentConfig (for deployment customization)
    • Excludes selector field (unused in OLMv0)
  • Runtime Schema Customization: The base schema is loaded and modified at runtime based on
    operator install modes: watchNamespace validation rules are customized per operator's supported install modes

  • Validation Infrastructure: Validation integrated into bundle config unmarshaling via config.UnmarshalConfig()

  • Regeneration Workflow: Added make update-registryv1-bundle-schema target to regenerate schema when
    upstream SubscriptionConfig changes.

When the upstream v1alpha1.SubscriptionConfig adds new fields (e.g., new k8s corev1 types), running the
regeneration target will automatically include them in the schema without manual updates.

Addresses OPRUN-4112 (Phase 1)

Future PRs will implement:

  • Phase 2: Renderer integration to apply deploymentConfig to Deployments
  • Phase 3: Provider integration to extract bundle configuration from bundles
  • Phase 4: Documentation

Copilot AI review requested due to automatic review settings January 20, 2026 19:51
@netlify
Copy link

netlify bot commented Jan 20, 2026

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit d5b6bc6
🔍 Latest deploy log https://app.netlify.com/projects/olmv1/deploys/697a3bd3e510050008cef5c7
😎 Deploy Preview https://deploy-preview-2454--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@anik120 anik120 requested review from tmshort and removed request for Copilot January 20, 2026 19:52
@anik120 anik120 requested review from joelanford and perdasilva and removed request for camilamacedo86 and thetechnick January 20, 2026 19:52
@anik120 anik120 force-pushed the deploymentconfig-json-schema branch from 1367534 to 2cd3619 Compare January 20, 2026 20:05
Copilot AI review requested due to automatic review settings January 20, 2026 20:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements Phase 1 of the DeploymentConfig feature to achieve feature parity with OLMv0's SubscriptionConfig for registry+v1 bundles. It adds automated JSON schema generation and validation infrastructure for the deploymentConfig field in ClusterExtension.

Changes:

  • Added reflection-based schema generator tool that introspects v1alpha1.SubscriptionConfig from the operator-framework/api package
  • Implemented embedded JSON schema validation infrastructure with comprehensive test coverage
  • Integrated deploymentConfig schema into bundle config validation with make target for regeneration

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
hack/tools/schema-generator/main.go Implements reflection-based generator that introspects Go types and parses AST to extract documentation, handling nested k8s types and inline fields
hack/tools/schema-generator/main_test.go Comprehensive test suite validating schema structure, selector field exclusion, and comparing generated vs committed schemas
internal/operator-controller/rukpak/bundle/schema/validator.go Validation infrastructure with embedded schema compilation and multi-format input support (JSON bytes, strings, structs)
internal/operator-controller/rukpak/bundle/schema/validator_test.go Test coverage for valid and invalid deployment configurations including edge cases
internal/operator-controller/rukpak/bundle/schema/deploymentconfig.json Generated JSON Schema Draft 7 document (1862 lines) defining all SubscriptionConfig fields except selector
internal/operator-controller/rukpak/bundle/schema/README.md Documentation explaining schema purpose, included/excluded fields, and regeneration workflow
internal/operator-controller/rukpak/bundle/registryv1.go Integration of deploymentConfig schema into bundle config validation
internal/operator-controller/config/config.go Added GetDeploymentConfig accessor method with nil handling
hack/tools/update-deploymentconfig-schema.sh Shell script for regenerating schema from vendor dependencies
Makefile Added update-deploymentconfig-schema make target

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@anik120 anik120 force-pushed the deploymentconfig-json-schema branch from 2cd3619 to 56d10e2 Compare January 20, 2026 20:26
Copilot AI review requested due to automatic review settings January 20, 2026 20:30
@anik120 anik120 force-pushed the deploymentconfig-json-schema branch from 56d10e2 to fce993a Compare January 20, 2026 20:30
@anik120 anik120 force-pushed the deploymentconfig-json-schema branch 2 times, most recently from cd32996 to 5bfc310 Compare January 20, 2026 20:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

// Write to file
if err := os.WriteFile(outputFile, data, 0600); err != nil {
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The file permission 0600 makes the generated schema file read/write for owner only. Since this is a JSON schema file that needs to be committed to the repository and read by the build process, it should use more permissive permissions like 0644 (readable by all, writable by owner).

Suggested change
if err := os.WriteFile(outputFile, data, 0600); err != nil {
if err := os.WriteFile(outputFile, data, 0644); err != nil {

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

0644 is what I started out with, but the linter fails with

hack/tools/schema-generator/main.go:106:12  gosec  G306: Expect WriteFile permissions to be 0600 or less

But if I change this to 0600, Copilot says it should be 0644.

The machines are fighting with each other, and I'm caught in between.

Copy link
Member

Choose a reason for hiding this comment

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

Use 644 and add a //nolint:gosec comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's only one user in the pod. There's no need to give group and other users access. 0600 has been used in many other places.

Copy link
Member Author

Choose a reason for hiding this comment

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

let's start off with 0600 and see if more is required in that case then

@anik120 anik120 force-pushed the deploymentconfig-json-schema branch from 5bfc310 to 64e7f6b Compare January 20, 2026 20:47
Copilot AI review requested due to automatic review settings January 20, 2026 21:01
@anik120 anik120 force-pushed the deploymentconfig-json-schema branch from 64e7f6b to 6b8cb00 Compare January 20, 2026 21:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Jan 20, 2026

Codecov Report

❌ Patch coverage is 58.16733% with 105 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.52%. Comparing base (8167ff8) to head (d5b6bc6).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
hack/tools/schema-generator/main.go 58.97% 72 Missing and 8 partials ⚠️
...al/operator-controller/rukpak/bundle/registryv1.go 50.00% 15 Missing and 4 partials ⚠️
internal/operator-controller/config/config.go 66.66% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2454      +/-   ##
==========================================
+ Coverage   69.49%   69.52%   +0.03%     
==========================================
  Files         101      102       +1     
  Lines        7754     8231     +477     
==========================================
+ Hits         5389     5723     +334     
- Misses       1930     2056     +126     
- Partials      435      452      +17     
Flag Coverage Δ
e2e 46.64% <33.92%> (+0.53%) ⬆️
experimental-e2e 13.43% <0.00%> (-0.03%) ⬇️
unit 57.55% <51.79%> (+0.36%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@perdasilva
Copy link
Contributor

perdasilva commented Jan 21, 2026

@anik120 have you considered approaching this by having the schema-generator generate the base registry+v1 bundle configuration schema including watchNamespace? Then the process of generating the final schema for a bundle could be:
deserialize the base schema, update it based on the bundle install mode configuration, then return that (or validate the configuration against that). This way we don't need to tie things down so closely to the deployment config.

Also, you could delete the deploymentConfig from the schema if the featuregate isn't enabled...

Makefile Outdated
env JQ=$(GOJQ) hack/tools/update-tls-profiles.sh

.PHONY: update-deploymentconfig-schema
update-deploymentconfig-schema: #EXHELP Update DeploymentConfig JSON schema by introspecting v1alpha1.SubscriptionConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

should we call it something like update-bundle-config-schema or somehow relate the naming of things back to bundle configuration? It might make it easier to understand if you're coming in fresh?

same with hack/tools/schema-generator

os.Exit(1)
}

fmt.Printf("Parsing SubscriptionConfig from %s...\n", subscriptionTypesFile)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering whether we need to track the subscriptionconfig at all. Do we think it will change any time soon? e.g. new fields added to it? if we didn't track it, would that reduce some of the complexity we're carrying?

Copy link
Contributor

Choose a reason for hiding this comment

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

This would probably be my last major concern. If we're convinced of we need to track subscriptionconfig, I think we're good.

Copy link
Member Author

@anik120 anik120 Jan 27, 2026

Choose a reason for hiding this comment

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

I'd originally suggested creating a brand new structure in the RFC too, one that won't track SubscirptionConfig, but Joe had other ideas https://docs.google.com/document/d/18O4qBvu5I4WIJgo5KU1opyUKcrfgk64xsI3tyXxmVEU/edit?disco=AAAByTBFx9c for maintainability

env JQ=$(GOJQ) hack/tools/update-tls-profiles.sh

.PHONY: update-registryv1-bundle-schema
update-registryv1-bundle-schema: #EXHELP Update registry+v1 bundle configuration JSON schema
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to run this as part of verify?

Copy link
Contributor

Choose a reason for hiding this comment

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

We'd want to call it verify-something-or-other.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added it to verify, didn't change the name from "update-...." though, since I see other targets that are similarly verb-ed there: update-tls-profiles, generate

Copilot AI review requested due to automatic review settings January 27, 2026 16:36
@anik120 anik120 force-pushed the deploymentconfig-json-schema branch from 6f5561c to c1e37bd Compare January 27, 2026 16:36
@anik120
Copy link
Member Author

anik120 commented Jan 27, 2026

@perdasilva @tmshort squashed the commits into one, this one's ready to go in now 👍🏽

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@anik120 anik120 force-pushed the deploymentconfig-json-schema branch from c1e37bd to 26cc593 Compare January 27, 2026 17:24
@tmshort
Copy link
Contributor

tmshort commented Jan 27, 2026

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 27, 2026
@@ -0,0 +1,25 @@
package schema
Copy link
Contributor

Choose a reason for hiding this comment

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

let's move all of this into registryv1.go, move the README.md to rukpak/bundle and remove the schema package. Just to keep it tight. Then I think we're good 🎉

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

…le configuration

Summary:

Implements Phase 1 of the variable configuration feature to achieve feature parity with
OLMv0's SubscriptionConfig for registry+v1 bundles.

RFC: https://docs.google.com/document/d/18O4qBvu5I4WIJgo5KU1opyUKcrfgk64xsI3tyXxmVEU/edit?tab=t.0#heading=h.x3tfh25grvnv

Details:

This PR adds infrastructure for JSON schema-based validation of registry+v1 bundle configuration
in ClusterExtension, including both `watchNamespace` and `deploymentConfig` fields:

- **Schema Generation Tool**: Parses SubscriptionConfig using AST to extract field names/types
  - Fetches Kubernetes OpenAPI v3 specs from GitHub releases
  - Maps fields to official Kubernetes schemas
  - Generates schema with `watchNamespace` (for operator scope) and `deploymentConfig` (for deployment customization)
  - Excludes `selector` field (unused in OLMv0)

- **Runtime Schema Customization**: The base schema is loaded and modified at runtime based on
operator install modes: `watchNamespace` validation rules are customized per operator's supported install modes

- **Validation Infrastructure**: Validation integrated into bundle config unmarshaling via `config.UnmarshalConfig()`

- **Regeneration Workflow**: Added `make update-registryv1-bundle-schema` target to regenerate schema when
upstream SubscriptionConfig changes.

When the upstream `v1alpha1.SubscriptionConfig` adds new fields (e.g., new k8s corev1 types), running the
regeneration target will automatically include them in the schema without manual updates.

Addresses OPRUN-4112 (Phase 1)

Future PRs will implement:
- Phase 2: Renderer integration to apply deploymentConfig to Deployments
- Phase 3: Provider integration to extract bundle configuration from bundles
- Phase 4: Documentation
Copilot AI review requested due to automatic review settings January 28, 2026 16:39
@anik120 anik120 force-pushed the deploymentconfig-json-schema branch from 26cc593 to d5b6bc6 Compare January 28, 2026 16:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@perdasilva perdasilva removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 28, 2026
@perdasilva
Copy link
Contributor

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 28, 2026
@perdasilva
Copy link
Contributor

I missed that Todd had already approved and removed what I thought was an approve =/ adding the lgtm them

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2026
Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci
Copy link

openshift-ci bot commented Jan 28, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: perdasilva, rashmigottipati, tmshort

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

The pull request process is described here

Details 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

@anik120 anik120 merged commit 12923b9 into operator-framework:main Jan 28, 2026
33 of 35 checks passed
anik120 added a commit to anik120/operator-controller that referenced this pull request Jan 29, 2026
This PR implements **Phase 2** of the [Deployment Configuration RFC](https://docs.google.com/document/d/18O4qBvu5I4WIJgo5KU1opyUKcrfgk64xsI3tyXxmVEU/edit?tab=t.0):
extending the OLMv1 bundle renderer to apply `DeploymentConfig` customizations to operator deployments.

Building on the foundation from operator-framework#2454, this PR enables the renderer to
accept and apply deployment configuration when rendering registry+v1 bundles.
The implementation follows OLMv0's behavior patterns to ensure compatibility and correctness.

The next PR will wire up the config in the `ClusterExtension` controller by parsing `spec.install.config`
to convert to `DeploymentConfig` and thread `DeploymentConfig` through the controller's render call chain
anik120 added a commit to anik120/operator-controller that referenced this pull request Jan 29, 2026
This PR implements **Phase 2** of the [Deployment Configuration RFC](https://docs.google.com/document/d/18O4qBvu5I4WIJgo5KU1opyUKcrfgk64xsI3tyXxmVEU/edit?tab=t.0):
extending the OLMv1 bundle renderer to apply `DeploymentConfig` customizations to operator deployments.

Building on the foundation from operator-framework#2454, this PR enables the renderer to
accept and apply deployment configuration when rendering registry+v1 bundles.
The implementation follows OLMv0's behavior patterns to ensure compatibility and correctness.

The next PR will wire up the config in the `ClusterExtension` controller by parsing `spec.install.config`
to convert to `DeploymentConfig` and thread `DeploymentConfig` through the controller's render call chain
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. crd-diff-override lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants