CNTRLPLANE-2777: feat(resource builder): allow to inject tls configuration into annotated config maps #1322
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,223 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| package resourcebuilder | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "errors" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "sort" | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| "sigs.k8s.io/kustomize/kyaml/yaml" | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| corev1 "k8s.io/api/core/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/apimachinery/pkg/labels" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/client-go/tools/cache" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/klog/v2" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/utils/clock" | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| configv1 "github.com/openshift/api/config/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| configclientv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/openshift/library-go/pkg/operator/configobserver/apiserver" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/openshift/library-go/pkg/operator/events" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/openshift/library-go/pkg/operator/resourcesynccontroller" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const ( | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // ConfigMapInjectTLSAnnotation is the annotation key that triggers TLS injection into ConfigMaps | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ConfigMapInjectTLSAnnotation = "config.openshift.io/inject-tls" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type optional[T any] struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| value T | ||||||||||||||||||||||||||||||||||||||||||||||||||
| found bool | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| type tlsConfig struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| minTLSVersion optional[string] | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cipherSuites optional[[]string] | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| func (b *builder) modifyConfigMap(ctx context.Context, cm *corev1.ConfigMap) error { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check for TLS injection annotation | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if value, ok := cm.Annotations[ConfigMapInjectTLSAnnotation]; !ok || value != "true" { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.V(2).Infof("ConfigMap %s/%s has %s annotation set to true", cm.Namespace, cm.Name, ConfigMapInjectTLSAnnotation) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Empty data, nothing to inject into | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if cm.Data == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.V(2).Infof("ConfigMap %s/%s has empty data, skipping TLS profile injection", cm.Namespace, cm.Name) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Observe TLS configuration from APIServer | ||||||||||||||||||||||||||||||||||||||||||||||||||
| tlsConf, err := b.observeTLSConfiguration(ctx, cm) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("unable to observe TLS configuration: %v", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| minTLSLog := "<not found>" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if tlsConf.minTLSVersion.found { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| minTLSLog = tlsConf.minTLSVersion.value | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cipherSuitesLog := "<not found>" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if tlsConf.cipherSuites.found { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cipherSuitesLog = fmt.Sprintf("%v", tlsConf.cipherSuites.value) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.V(4).Infof("ConfigMap %s/%s: observed minTLSVersion=%v, cipherSuites=%v", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cm.Namespace, cm.Name, minTLSLog, cipherSuitesLog) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Process each data entry that contains GenericOperatorConfig | ||||||||||||||||||||||||||||||||||||||||||||||||||
| for key, value := range cm.Data { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. This was supposed to be a comment about the need to document this functionality in the enhancement repository. However, you already thought of this on Slack, awesome! I will have a look for a right page. I am leaving this comment for me to remember this.
Contributor
There was a problem hiding this comment. I think that we can create a new document in https://github.com/openshift/enhancements/tree/master/dev-guide/cluster-version-operator/dev, as this will not be an enhancement per se, but rather a dev-guide documentation explaining the usage and any constraints. |
||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.V(4).Infof("Processing %q key", key) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Parse YAML into RNode to preserve formatting and field order | ||||||||||||||||||||||||||||||||||||||||||||||||||
| rnode, err := yaml.Parse(value) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.V(4).Infof("ConfigMap's %q entry parsing failed: %v", key, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Not valid YAML, skip this entry | ||||||||||||||||||||||||||||||||||||||||||||||||||
ingvagabund marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| continue | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if this is a supported config kind | ||||||||||||||||||||||||||||||||||||||||||||||||||
| switch { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| case rnode.GetKind() == "GenericOperatorConfig" && rnode.GetApiVersion() == operatorv1alpha1.GroupVersion.String(): | ||||||||||||||||||||||||||||||||||||||||||||||||||
| case rnode.GetKind() == "GenericControllerConfig" && rnode.GetApiVersion() == configv1.GroupVersion.String(): | ||||||||||||||||||||||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.V(4).Infof("ConfigMap's %q entry is not a supported config type. Only GenericOperatorConfig (%v) and GenericControllerConfig (%v) are. Skipping this entry", key, operatorv1alpha1.GroupVersion.String(), configv1.GroupVersion.String()) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| continue | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.V(2).Infof("ConfigMap %s/%s processing GenericOperatorConfig in key %s", cm.Namespace, cm.Name, key) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Inject TLS settings into the GenericOperatorConfig while preserving structure | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if err := updateRNodeWithTLSSettings(rnode, tlsConf); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("failed to inject the TLS configuration: %v", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Marshal the modified RNode back to YAML | ||||||||||||||||||||||||||||||||||||||||||||||||||
| modifiedYAML, err := rnode.String() | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("failed to marshall the modified ConfigMap back to YAML: %v", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Update the ConfigMap data entry with the modified YAML | ||||||||||||||||||||||||||||||||||||||||||||||||||
| cm.Data[key] = modifiedYAML | ||||||||||||||||||||||||||||||||||||||||||||||||||
| klog.V(2).Infof("ConfigMap %s/%s updated GenericOperatorConfig with TLS profile in key %s", cm.Namespace, cm.Name, key) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // observeTLSConfiguration retrieves TLS configuration from the APIServer cluster CR | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // using ObserveTLSSecurityProfile and extracts minTLSVersion and cipherSuites. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| func (b *builder) observeTLSConfiguration(ctx context.Context, cm *corev1.ConfigMap) (*tlsConfig, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Create a lister adapter for ObserveTLSSecurityProfile | ||||||||||||||||||||||||||||||||||||||||||||||||||
| lister := &apiServerListerAdapter{ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| client: b.configClientv1.APIServers(), | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx: ctx, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| listers := &configObserverListers{ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| apiServerLister: lister, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Create an in-memory event recorder that doesn't send events to the API server | ||||||||||||||||||||||||||||||||||||||||||||||||||
| recorder := events.NewInMemoryRecorder("configmap-tls-injection", clock.RealClock{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Call ObserveTLSSecurityProfile to get TLS configuration | ||||||||||||||||||||||||||||||||||||||||||||||||||
| observedConfig, errs := apiserver.ObserveTLSSecurityProfile(listers, recorder, map[string]any{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(errs) > 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, fmt.Errorf("error observing TLS profile for ConfigMap %s/%s: %w", cm.Namespace, cm.Name, errors.Join(errs...)) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| config := &tlsConfig{} | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Extract minTLSVersion from the observed config | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if minTLSVersion, minTLSFound, err := unstructured.NestedString(observedConfig, "servingInfo", "minTLSVersion"); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if minTLSFound { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| config.minTLSVersion = optional[string]{value: minTLSVersion, found: true} | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Extract cipherSuites from the observed config | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if cipherSuites, ciphersFound, err := unstructured.NestedStringSlice(observedConfig, "servingInfo", "cipherSuites"); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if ciphersFound { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Sort cipher suites for consistent ordering | ||||||||||||||||||||||||||||||||||||||||||||||||||
| sort.Strings(cipherSuites) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| config.cipherSuites = optional[[]string]{value: cipherSuites, found: true} | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| return config, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // updateRNodeWithTLSSettings injects TLS settings into a GenericOperatorConfig RNode while preserving structure. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // If a field in tlsConf is not found, the corresponding field will be deleted from the RNode. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| func updateRNodeWithTLSSettings(rnode *yaml.RNode, tlsConf *tlsConfig) error { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| servingInfo, err := rnode.Pipe(yaml.LookupCreate(yaml.MappingNode, "servingInfo")) | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. I have seen your comment. Feel free to add such logic. Something like this should hopefully work:
Suggested change
with the updated unit test and a new test case/s for this new logic.
Contributor
There was a problem hiding this comment. I am not aware of the impact of the existing logic. If it's a no-op, we can leave it as is due to time constraints. If we are not certain, we can implement the logic.
Member
Author
There was a problem hiding this comment. Minor issue. Given both fields are always present this is no-op. Please ignore. |
||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle cipherSuites field | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if tlsConf.cipherSuites.found { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| seqNode := yaml.NewListRNode(tlsConf.cipherSuites.value...) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if err := servingInfo.PipeE(yaml.SetField("cipherSuites", seqNode)); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if err := servingInfo.PipeE(yaml.Clear("cipherSuites")); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle minTLSVersion field | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if tlsConf.minTLSVersion.found { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if err := servingInfo.PipeE(yaml.SetField("minTLSVersion", yaml.NewStringRNode(tlsConf.minTLSVersion.value))); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if err := servingInfo.PipeE(yaml.Clear("minTLSVersion")); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // apiServerListerAdapter adapts a client interface to the lister interface | ||||||||||||||||||||||||||||||||||||||||||||||||||
| type apiServerListerAdapter struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| client configclientv1.APIServerInterface | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx context.Context | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| func (a *apiServerListerAdapter) List(selector labels.Selector) ([]*configv1.APIServer, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Not implemented - ObserveTLSSecurityProfile only uses Get() | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| func (a *apiServerListerAdapter) Get(name string) (*configv1.APIServer, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return a.client.Get(a.ctx, name, metav1.GetOptions{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| // configObserverListers implements the configobserver.Listers interface. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // It's expected to be used solely for apiserver.ObserveTLSSecurityProfile. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| type configObserverListers struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| apiServerLister configlistersv1.APIServerLister | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| func (l *configObserverListers) APIServerLister() configlistersv1.APIServerLister { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return l.apiServerLister | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| func (l *configObserverListers) ResourceSyncer() resourcesynccontroller.ResourceSyncer { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Not needed for TLS observation | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| func (l *configObserverListers) PreRunHasSynced() []cache.InformerSynced { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| // Not needed for TLS observation | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+215
to
+223
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: # First, let's understand the repo structure and find the relevant files
git ls-files | grep -E "(resourcebuilder|configobserver)" | head -20Repository: openshift/cluster-version-operator Length of output: 1188 🏁 Script executed: # Read the core.go file to see the full context
cat -n lib/resourcebuilder/core.go | sed -n '220,240p'
# Also check the interface definition
cat -n lib/resourcebuilder/interface.go | head -50
# Look at the observe_tlssecurityprofile implementation
cat -n vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_tlssecurityprofile.goRepository: openshift/cluster-version-operator Length of output: 8005 🏁 Script executed: # Find the APIServerLister interface definition
rg -n "type APIServerLister" --type go
# Search for all uses of ResourceSyncer in the codebase
rg -n "ResourceSyncer" --type go
# Search for all uses of PreRunHasSynced in the codebase
rg -n "PreRunHasSynced" --type go
# Find where configObserverListers is instantiated
rg -n "configObserverListers{" --type goRepository: openshift/cluster-version-operator Length of output: 2179 🏁 Script executed: # Read the config_observer_controller.go around the lines that call PreRunHasSynced
cat -n vendor/github.com/openshift/library-go/pkg/operator/configobserver/config_observer_controller.go | sed -n '35,45p'
# Read the context around lines 255-257 where PreRunHasSynced is called
cat -n vendor/github.com/openshift/library-go/pkg/operator/configobserver/config_observer_controller.go | sed -n '250,270p'
# Look at the full Listers interface definition
cat -n vendor/github.com/openshift/library-go/pkg/operator/configobserver/config_observer_controller.go | sed -n '33,50p'
# Check the apiserver listers.go to understand the interface
cat -n vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/listers.goRepository: openshift/cluster-version-operator Length of output: 3078 🏁 Script executed: # Find where listersToInformer is called
rg -n "listersToInformer" --type go
# Look for the Run method or startup code that uses this
cat -n vendor/github.com/openshift/library-go/pkg/operator/configobserver/config_observer_controller.go | sed -n '200,250p'
# Check how configObserverListers is initialized and used in lib/resourcebuilder/core.go
cat -n lib/resourcebuilder/core.go | sed -n '1,50p'
# Look for where configObserverListers is created/instantiated
rg -n "New.*configObserverListers\|&configObserverListers" --type goRepository: openshift/cluster-version-operator Length of output: 6154 🏁 Script executed: # Look at the code around line 126 in config_observer_controller.go
cat -n vendor/github.com/openshift/library-go/pkg/operator/configobserver/config_observer_controller.go | sed -n '100,140p'
# Search for any calls to ResourceSyncer() in the codebase
rg -n "\.ResourceSyncer\(\)" --type go -A 2
# Check if the configObserverListers is actually used with the config observer controller
grep -r "configObserverListers\|NewConfigObserverListers" lib/ --include="*.go"Repository: openshift/cluster-version-operator Length of output: 2578 🏁 Script executed: # Find where configObserverListers is used
rg -n "configObserverListers" lib/resourcebuilder/core.go -B 5 -A 10
# Check if it's passed to NewNestedConfigObserver or similar
rg -n "NewNestedConfigObserver\|NewConfigObserver" lib/ --include="*.go" -B 2 -A 2Repository: openshift/cluster-version-operator Length of output: 2026 🏁 Script executed: # Double-check the current usage is safe by seeing what ObserveTLSSecurityProfile actually does
cat -n vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_tlssecurityprofile.go | sed -n '35,80p'Repository: openshift/cluster-version-operator Length of output: 2460 Return proper implementations instead of
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the existing logic, I am proposing to add small tests for:
TestModifyConfigMap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update the PR tomorrow