Skip to content

Conversation

@anik120
Copy link
Member

@anik120 anik120 commented Feb 3, 2026

Summary

Completes Phase 3 of the Deployment Configuration RFC. Extracts deploymentConfig from validated ClusterExtension configuration and passes it to the bundle renderer.

Builds on:

This completes the RFC - users can now customize operator deployments via ClusterExtension.spec.config.inline:

config:
  configType: Inline
  inline:
    watchNamespace: "some-namespace"
    deploymentConfig:
      env:
        - name: TEST_ENV
          value: test-value
      nodeSelector:
        kubernetes.io/os: linux

Description

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

Copilot AI review requested due to automatic review settings February 3, 2026 17:08
@openshift-ci openshift-ci bot requested review from pedjak and thetechnick February 3, 2026 17:08
@netlify
Copy link

netlify bot commented Feb 3, 2026

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit d7cb702
🔍 Latest deploy log https://app.netlify.com/projects/olmv1/deploys/698242600407bf000809d3b1
😎 Deploy Preview https://deploy-preview-2482--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 perdasilva and tmshort and removed request for pedjak and thetechnick February 3, 2026 17:10
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 completes Phase 3 of the Deployment Configuration RFC, enabling users to customize operator deployments through the ClusterExtension spec. It extracts validated deploymentConfig from the bundle configuration and passes it to the renderer for application to operator deployments.

Changes:

  • Added extraction logic to get deploymentConfig from validated bundle configuration
  • Implemented conversion from map to DeploymentConfig struct via JSON marshaling
  • Added comprehensive test coverage for various deploymentConfig scenarios

Reviewed changes

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

File Description
internal/operator-controller/applier/provider.go Added deploymentConfig extraction and conversion logic in Get method; implemented convertToDeploymentConfig helper function
internal/operator-controller/applier/provider_test.go Added 4 test cases covering deploymentConfig with single field, no config, multiple fields, and combination with watchNamespace

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

@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 74.19355% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.76%. Comparing base (e41eb44) to head (d7cb702).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
internal/operator-controller/applier/provider.go 74.19% 4 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2482      +/-   ##
==========================================
- Coverage   73.73%   69.76%   -3.97%     
==========================================
  Files         102      102              
  Lines        8451     8473      +22     
==========================================
- Hits         6231     5911     -320     
- Misses       1741     2090     +349     
+ Partials      479      472       -7     
Flag Coverage Δ
e2e 46.01% <45.16%> (-0.29%) ⬇️
experimental-e2e 12.99% <0.00%> (-41.12%) ⬇️
unit 57.86% <74.19%> (+0.03%) ⬆️

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.

@anik120 anik120 force-pushed the config-api-controller-integration branch from 1d4703f to 278d27f Compare February 3, 2026 18:31
**Summary**

Completes Phase 3 of the [Deployment Configuration RFC](https://docs.google.com/document/d/1bDo3W1asZqjJTgZy7BcVOGtKOEukp0vUi5CO1ac3vwc/edit?usp=sharing).
Extracts `deploymentConfig` from validated ClusterExtension
configuration and passes it to the bundle renderer.

Builds on:
- (PR2454)[operator-framework#2454]: Config API and JSON schema validation
- (PR2469)[operator-framework#2469]: Renderer support for applying deployment customizations

This completes the RFC - users can now customize operator deployments via
ClusterExtension.spec.config.inline:

```
config:
  configType: Inline
  inline:
    watchNamespace: "some-namespace"
    deploymentConfig:
      env:
        - name: TEST_ENV
          value: test-value
      nodeSelector:
        kubernetes.io/os: linux
```
@anik120 anik120 force-pushed the config-api-controller-integration branch from 278d27f to d7cb702 Compare February 3, 2026 18:45
Copilot AI review requested due to automatic review settings February 3, 2026 18:45
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 2 out of 2 changed files in this pull request and generated no new comments.


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

if err != nil {
return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("invalid deploymentConfig: %w", err))
}
opts = append(opts, render.WithDeploymentConfig(deploymentConfig))
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when deploymentConfig is nil (i.e. first return case. of return nil, nil?

ResourceGenerators: []render.ResourceGenerator{
func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
t.Log("ensure deploymentConfig is nil for empty config object")
require.Nil(t, opts.DeploymentConfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

Although, you do seem to test it here.

@tmshort
Copy link
Contributor

tmshort commented Feb 3, 2026

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2026
@perdasilva
Copy link
Contributor

/approve

@openshift-ci
Copy link

openshift-ci bot commented Feb 3, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: perdasilva

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2026
@anik120 anik120 merged commit e274216 into operator-framework:main Feb 3, 2026
32 of 35 checks passed
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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants