Copilot commented on code in PR #11149:
URL: https://github.com/apache/gravitino/pull/11149#discussion_r3279824713
##########
docs/gravitino-server-config.md:
##########
@@ -312,6 +312,7 @@ Below is a list of catalog properties that will be used by
all Gravitino catalog
| `package` | The path of the catalog package, Gravitino leverages
this path to load the related catalog libs and configurations. The package
should consist two folders, `conf` (for catalog related configurations) and
`libs` (for catalog related dependencies/jars) | (none) | No |
0.5.0 |
| `cloud.name` | The property to specify the cloud that the catalog is
running on. The valid values are `aws`, `azure`, `gcp`, `on_premise` and
`other`.
| (none) | No |
0.6.0-incubating |
| `cloud.region-code` | The property to specify the region code of the cloud
that the catalog is running on.
| (none) | No |
0.6.0-incubating |
+| `gravitino.catalog.credential.backfillToProperties` | For backward
compatibility only: if `true`, the server exposes hidden catalog credentials in
plain catalog properties so that connectors that do not support credential
vending can still read them. **Enabling this is a security risk** — credentials
are visible to anyone who can read catalog properties. Disable once all
connectors are upgraded. | `false` | No | 0.9.0 |
Review Comment:
`gravitino.catalog.credential.backfillToProperties` is a server-level
configuration (added in Configs.java) rather than a per-catalog property users
can set in CREATE/ALTER CATALOG. Documenting it in the "catalog properties"
table can mislead readers into thinking it's a catalog property; consider
moving it to an appropriate server configuration section (or clearly labeling
it as server config).
##########
catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogCredential.java:
##########
@@ -31,12 +35,18 @@
import org.apache.gravitino.credential.JdbcCredential;
import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.CatalogEntity;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
/** Tests for JdbcCatalog credential provider functionality. */
public class TestJdbcCatalogCredential {
+ @AfterEach
+ void resetGravitinoEnvConfig() throws IllegalAccessException {
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "config", null, true);
Review Comment:
`@AfterEach` resets the global GravitinoEnv singleton's `config` field to
null. If another test class previously initialized GravitinoEnv (and expects it
to remain initialized), this can cause cross-test interference and flaky
failures depending on execution order. Consider saving the previous `Config`
value before mutating it in this test class and restoring it in `@AfterEach`
instead of always nulling it out.
--
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]
