feat: add passthrough config mechanism for Connect, Package Manager, and Workbench

[ian-flores]

Summary

• Adds escape-hatch passthrough fields to all three product configs (Connect, Package Manager, Workbench) so users can set arbitrary config values without waiting for operator releases
• Connect & PM: additionalConfig string, raw content appended after the generated config
• Workbench: additionalConfigs map with config file name keys, raw content appended per file
• Site CRD propagation for all three products
• Fixes pre-existing typo: WorkbenchLauncherKubnernetesResourcesConfigSection → WorkbenchLauncherKubernetesResourcesConfigSection

Motivation

The operator currently defines ~25-33% of each product's config values as typed Go struct fields. Adding a new setting requires modifying 4+ files, running codegen, and releasing a new operator version. This passthrough mechanism lets users set any config value immediately.

Design

Connect & Package Manager (gcfg format)

config:
  Server:
    Address: "typed-value.com"
  additionalConfig: |
    [Server]
    DataDir = /custom/path
    [NewSection]
    NewKey = any-value

• Raw gcfg content appended after the generated config
• gcfg parsing naturally resolves conflicts: last occurrence wins for scalar values, list values are combined
• Consistent with how Workbench handles passthrough

Workbench (multiple config files)

config:
  additionalConfigs:
    "rserver.conf": |
      session-timeout-minutes=120
      custom-setting=value
    "rsession.conf": |
      r-cran-repos=https://custom.repo.com
  additionalSessionConfigs:
    "repos.conf": |
      CRAN=https://custom.cran.com

• Keys are config file names
• Content appended after generated config
• Per-file approach handles Workbench's multiple formats (INI, DCF, JSON, newline-delimited)

Test plan

• Connect: string append, override via gcfg last-write-wins, empty string (3 tests)
• Package Manager: string append, empty string (2 tests)
• Workbench: append to existing, new file, empty, nil — for both IniConfig and SessionIniConfig (7 tests)
• All existing tests pass (go test ./api/core/v1beta1/...)
• envtest integration tests (require local kubebuilder — pre-existing setup gap)
• Deploy to test cluster and verify config files are generated correctly

[claude]

Claude finished @ian-flores's task —— View job

---

Review Complete

• Read review guidelines
• Create pending review
• Analyze PR diff and changed files
• Review API changes (Connect, Package Manager, Workbench configs)
• Review Controller changes
• Review generated code
• Submit review

Submitted a review with 2 issues to address:

1. Non-deterministic map iteration in Connect/PM config generation - sort keys for consistent output
2. Potential nil pointer dereference in Workbench controller when propagating additional configs

Overall, the passthrough mechanism is well-designed with good test coverage.

[claude]

Review Summary

This PR implements a well-designed passthrough config mechanism for all three products. The design choices are sound: Section.Key format for gcfg products (Connect/PM) and file-based appending for Workbench's multiple config formats.

Issues to Address

1. Non-deterministic map iteration (Connect & Package Manager)

• api/core/v1beta1/connect_config.go:310 and api/core/v1beta1/package_manager_config.go:98
• Iterating over Additional map has non-deterministic ordering in Go
• When multiple keys target the same new section, their order within that section will vary between runs
• Suggestion: Sort the keys before processing:

keys := make([]string, 0, len(configStruct.Additional))
for key := range configStruct.Additional {
    keys = append(keys, key)
}
sort.Strings(keys)
for _, key := range keys {
    // process
}

2. Potential nil pointer dereference (Workbench controller)

• internal/controller/core/site_controller_workbench.go:402 and :407
• If targetWorkbench.Spec.Config.WorkbenchIniConfig or WorkbenchSessionIniConfig is nil, the assignment will panic
• Suggestion: Add nil guards:

if site.Spec.Workbench.AdditionalConfigs != nil {
    if targetWorkbench.Spec.Config.WorkbenchIniConfig == nil {
        targetWorkbench.Spec.Config.WorkbenchIniConfig = &v1beta1.WorkbenchIniConfig{}
    }
    targetWorkbench.Spec.Config.WorkbenchIniConfig.AdditionalConfigs = site.Spec.Workbench.AdditionalConfigs
}

Positives

• Good test coverage: Edge cases like malformed keys, empty maps, nil maps, and override scenarios are well tested
• Typo fix: WorkbenchLauncherKubnernetesResourcesConfigSection → WorkbenchLauncherKubernetesResourcesConfigSection is a good cleanup
• Consistent naming: AdditionalConfig for single-file products vs AdditionalConfigs/AdditionalSessionConfigs for Workbench makes the behavior clear
• SplitN(..., 2) approach: Correctly handles multi-dot keys like Good.Section.Key
• Shared sectionEntry pattern: Well-structured for maintaining key ordering; duplication between Connect/PM is acceptable for now

Notes

• The Workbench AdditionalConfigs map iteration (workbench_config.go:173) also has non-deterministic order, but since each filename is independent (not affecting order within a file), this is lower priority
• CRD updates, deepcopy, and client-go generated code all look correct

[statik]

thanks for creating this! we have long felt the pain of needing this escape hatch. I think this would unblock @brendanhcullen, who needs support for setting Connect OAuth2.RegisterOnFirstLogin = false on academy training sites.

[Lytol]

I'm curious to hear what you think about my comment regarding just doing string-based config appending vs. the dot-notated keys. If you disagree, I'll defer to you and happily approve.

One other nit, but super subjective. It's a little odd to use additional vs additionalConfigs between PM/Connect and Workbench. I get that they're slightly different because Workbench is multiple config files (and expects file keys because of that), but I personally gravitate toward keeping them as the same key name. 🤷 Ridiculously subjective, so I defer to you.

[Lytol]

For Workbench, you key the file names and then the actual config values are multi-line strings that get appended. I think it represents KISS wonderfully and there's no cleverness/complexity to worry about. Why not do the same for the Package Manager and Connect configs? Skip the file keys (since there's only a single config), but still just have the value as a multi-line string and append to the end of the generated config. You'd get to skip all the code complexity in here with ordering, de-duping, etc. And it "just works" because of how the parsing of the gcfg works... it'll automatically combine "list" values, and it'll keep latest for scalar values. There's cleverness in doing the keys with dot-notation, but I think it makes things harder rather than easier. 🤷

[ian-flores]

Switched both Connect and PM to string appending, matching the Workbench pattern. Deleted ~300 lines of section-merge logic in the process.

Also took the naming nit: renamed additional → additionalConfig across the config structs and JSON tags to align with the existing site-level field names.

[Lytol]

❤️ This will make life so much easier!