feat: remove product config & clean up#842
Open
maltesander wants to merge 13 commits into
Open
Conversation
Point operator-rs to the smooth-operator branch (which exposes the v2:: framework module) and bump tokio to 1.52 and clap to 4.6. This is the foundation for removing the product-config dependency; existing product-config code still compiles and all tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rewrite the validate step to produce a typed `ValidatedCluster` (name, image, role_group_configs) instead of the product-config `ValidatedRoleConfigByPropertyKind`. OPA never used product-config for actual config content: `compute_env/cli/files` all returned empty maps, so the validated role config only served to enumerate role groups and to feed the DaemonSet's env vars (which were just merged `envOverrides`). - validate() no longer takes a ProductConfigManager; it merges config via the existing `OpaCluster::merged_config()` and resolves the cluster name into the v2 `ClusterName` type. - reconcile_opa iterates `role_group_configs` and sources the merged config from there. - The DaemonSet builder drops the `server_config` PropertyNameKind map parameter and merges role/role-group `envOverrides` inline, mirroring how `cliOverrides` were already merged. - Remove ProductConfigManager from Ctx and main.rs; the CLI argument is kept but ignored for backwards compatibility. - Derive Ord/PartialOrd on OpaRole so it can key the role_group_configs BTreeMap. The product-config crate dependency and the now-unused Configuration / KeyValueOverridesProvider impls are removed in a follow-up commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the rolegroup ConfigMap and `config.json` building out of the controller into a dedicated `controller/build/config_map.rs`. The build step now takes the `ValidatedCluster` and only uses the `OpaCluster` for the owner reference and object metadata, no longer reading `spec` for config content. To carry the merged `configOverrides` through `ValidatedCluster`, migrate `OpaConfigOverrides.config_json` from the non-mergeable `config_overrides::JsonConfigOverrides` to the v2 `JsonOrKeyValueConfigOverrides`, which: - implements `Merge`, so role- and role-group-level overrides are merged once in the validate step (role group wins) and stored on `OpaRoleGroupConfig`; - preserves the previous apply order (role patch first, then role group on top) via the v2 `JsonConfigOverrides` sequence; - applies infallibly, removing the config-override error path; - is CRD-safe (raw object schema), unlike the v2 `JsonConfigOverrides` enum whose `JsonPatch` variant cannot be rendered into a structural CRD schema. `ValidatedCluster` gains a `cluster_config` holding `user_info` so the ConfigMap build no longer needs the raw spec for `user-info-fetcher.json`. Add unit tests covering the default `config.json` and role/role-group override precedence. Regenerate extra/crds.yaml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the now-dead `impl Configuration for OpaConfigFragment` and `impl KeyValueOverridesProvider for OpaConfigOverrides` (both no-ops kept only to satisfy the product-config pipeline, which is no longer used), and remove the `product-config` crate from the workspace and operator-binary manifests. OPA never derived real config from product-config, so nothing is lost. The empty `deploy/config-spec/properties.yaml` is kept (it is still mounted by the Docker image and Helm chart) and normalized to match the other operators. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Introduce `controller/build/properties/` with one build step per config file and a `ConfigFileName` enum, mirroring trino/hdfs (adapted for OPA's JSON files, so the builders return serialized `String`s and there is no key-value writer). - `properties/config_json.rs`: the `config.json` builder (the former `build_config_file` plus the `OpaClusterConfigFile` struct family) with its own error type and the default/override-precedence unit tests. - `properties/user_info_fetcher.rs`: serializes `user-info-fetcher.json`. - `properties/mod.rs`: the `ConfigFileName` enum and a shared `test_support::validated_cluster_from_spec` helper. - `config_map.rs` shrinks to metadata + dispatch + assembly, wrapping the per-file builder errors. The Vector config file keeps coming from the existing logging call for now; it is folded into the build step in a follow-up commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the Vector config building and the `BundleBuilderLogLevel` mapping out of the top-level `product_logging` module into `controller/build/properties/logging.rs`, mirroring hdfs. `build_vector_config` returns `Option<String>` (the agent config or `None` when disabled) and is infallible, so the dead `InvalidLoggingConfig` error path is dropped. The DaemonSet now imports `BundleBuilderLogLevel` from the build module, and the standalone `product_logging.rs` is removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move discovery.rs into controller/build/discovery.rs and refactor build_discovery_config_map to take the ValidatedCluster + role Service + cluster_info, reading the TLS scheme/port and secret class from cluster_config and the app version from the resolved image. The owner object is used only for the owner reference and object metadata. ValidatedClusterConfig gains a `tls` field (alongside `user_info`), mirroring trino/hdfs. The discovery ConfigMap name now comes from `cluster.name`, which finally consumes that field and removes its `#[allow(dead_code)]`. Adds unit tests asserting the discovery URL (http/https + port) and the OPA_SECRET_CLASS entry with and without TLS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
16 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker