markhoerth opened a new pull request, #11268:
URL: https://github.com/apache/gravitino/pull/11268

   Bundles seven items from the chart enterprise readiness audit. All items are 
small-scope, none depend on each other, and the bundle deliberately excludes 
the items tracked separately.
   
   ## Items in scope
   
   - **Item 2**: Remove `gravitino.datastrato.*` property references from 
`ci-values.yaml` and the commented examples in `values.yaml`. Vendor-neutral 
example names used so the `visibleConfigs` mechanism remains documented and 
exercised in CI.
   - **Item 3**: Add structured `values.yaml` paths for 
`entity.maxConnections`, `lock.maxNodes`, and `lock.minNodes`. Properties used 
by the production sample in `docs/gravitino-server-config.md#production` that 
previously required `additionalConfigItems` workarounds. Emitted conditionally 
from the `gravitino.conf` template so server defaults apply when unset.
   - **Item 5**: Ship `dev-values.yaml` and `prod-values.yaml` scenario files 
derived from the development and production samples in 
`gravitino-server-config.md`. The prod scenario uses structured paths for the 
properties added by item 3 (no `additionalConfigItems`).
   - **Item 6**: Replace `NOTES.txt` with a post-install deployment summary 
that surfaces the actual backend, persistence, IRC, and auth configuration. 
Adds warnings for ephemeral H2 metadata, in-memory IRC backend, and simple-mode 
authentication.
   - **Item 7**: Remove `testAdditionalConfigItems` test-fixture properties 
from `ci-values.yaml`. Replaced with a realistic 
`gravitino.eventListener.names` example.
   - **Item 8**: Default-credential warnings above the `mysql` and `postgresql` 
sections in `values.yaml`. `NOTES.txt` fires runtime warnings when in-chart 
databases are enabled with default passwords.
   - **Item 9**: Emit dynamic-config-provider sub-properties (`gravitino-uri`, 
`gravitino-metalake`, `default-catalog-name`) conditionally, so unset values do 
not render as empty-string property assignments.
   
   ## Items deliberately out of scope
   
   - **Item 1 (Kubernetes Secret references for sensitive values)**: P0 RBC 
blocker. Touches `templates/deployment.yaml` and flips `SKIP_CONFIG_REWRITE`. 
Tracked as a separate PR.
   - **Item 4 (`existingConfigMap` support)**: Deferred. Niche GitOps unlock; 
no current customer pressure.
   - **Item 10 (OIDC OAuth values schema)**: In progress on a separate branch.
   
   ## Validation
   
   - `helm lint` clean against defaults and both new scenario files 
(`dev-values.yaml`, `prod-values.yaml`).
   - `helm template` renders expected `gravitino.conf` content for default, 
dev, and prod scenarios.
   - Item 3's structured paths verified: emit correctly when set (prod scenario 
produces `gravitino.entity.store.relational.maxConnections = 200`, 
`gravitino.lock.maxNodes = 500000`, `gravitino.lock.minNodes = 5000`), omit 
cleanly when unset.
   - Item 9's conditional emission verified: dev scenario produces a single 
`gravitino.iceberg-rest.gravitino-metalake = test` line with no empty 
`gravitino-uri =` or `default-catalog-name =` lines.
   - `NOTES.txt` warnings verified via `helm install --dry-run` for H2 
ephemeral, IRC in-memory, simple auth, and default MySQL/PostgreSQL credentials.
   - Vendor branding and test-fixture cleanups verified: `grep -r datastrato 
dev/charts/gravitino/` and `grep -r testAdditionalConfigItems 
dev/charts/gravitino/` both return no results.
   - Prod scenario uses only structured paths: `grep additionalConfigItems 
dev/charts/gravitino/resources/scenarios/prod-values.yaml` returns no results.
   
   ## Runtime validation
   
   Runtime validation is **blocked by #11267** (chart-vs-image path mismatch in 
all currently published Gravitino images). The chart hardcodes `/opt/gravitino` 
but published images (`1.2.0` and `1.3.0-SNAPSHOT` both verified) install 
Gravitino at `/root/gravitino`. A default `helm install` against `main` fails 
with `CrashLoopBackOff` identically to this branch, so the blocker is not 
introduced by this PR.
   
   The path bug has a second layer where `/root/` has 700 permissions 
incompatible with the chart's non-root container security context 
(`runAsNonRoot: true, runAsUser: 1000`). Any chart-side workaround requires 
disabling `runAsNonRoot`, which is a security regression unacceptable for 
enterprise deployments. This is why Option A in #11267 (move the image install 
location to `/opt/gravitino`) is the only viable resolution.
   
   Once #11267 is resolved, this PR's edits will runtime-validate without 
modification since none of them touch the deployment template or path 
assumptions.
   
   ## Documentation followup
   
   Once this PR and the separate item 1 PR (Secret references) land, 
`docs/chart.md` can describe a complete install-to-production flow including 
the dev and prod scenario files. The scenario files in `resources/scenarios/` 
are the canonical source the docs should link via raw GitHub URLs.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to