Copilot commented on code in PR #11213:
URL: https://github.com/apache/gravitino/pull/11213#discussion_r3332114005
##########
.github/workflows/multi-instance-consistency-test.yml:
##########
@@ -0,0 +1,220 @@
+name: Multi-Instance Consistency Test
+
+# Verifies that authorization state (owners, roles, role privileges, user-role
+# grants) propagates correctly between two Gravitino instances backed by the
+# same MySQL entity store. Both instances run with the entity cache disabled
+# and authorization enabled, so the only consistency contracts under test are
+# the JcasbinAuthorizer caches and the JcasbinChangePoller invalidation paths.
+#
+# The test plan and assertions live in
dev/ci/test_multi_instance_consistency.sh.
+
+on:
+ push:
+ branches: [ "main", "branch-*" ]
+ pull_request:
+ branches: [ "main", "branch-*" ]
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number ||
github.ref }}
+ cancel-in-progress: true
+
+jobs:
+ changes:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+ - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36
+ id: filter
+ with:
+ filters: |
+ authz_changes:
+ - .github/workflows/multi-instance-consistency-test.yml
+ - api/**
+ - common/**
+ - core/**
+ - server/**
+ - server-common/**
+ - conf/**
+ - scripts/mysql/**
+ - dev/ci/test_multi_instance_consistency.sh
+ - build.gradle.kts
+ - gradle.properties
+ - gradle/**
+ - settings.gradle.kts
+ outputs:
+ authz_changes: ${{ steps.filter.outputs.authz_changes }}
+
+ multi-instance-consistency:
+ needs: changes
+ if: needs.changes.outputs.authz_changes == 'true'
+ runs-on: ubuntu-latest
+ timeout-minutes: 30
+ services:
+ mysql:
+ image: mysql:8.0.33
+ env:
+ MYSQL_ROOT_PASSWORD: gravitino
+ MYSQL_DATABASE: gravitino
+ MYSQL_USER: gravitino
+ MYSQL_PASSWORD: gravitino
+ ports:
+ - 3306:3306
+ options: >-
+ --health-cmd="mysqladmin ping -uroot -pgravitino --silent"
+ --health-interval=5s
+ --health-timeout=3s
+ --health-retries=20
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: actions/setup-java@v4
+ with:
+ java-version: 17
+ distribution: 'temurin'
+ cache: 'gradle'
+
+ - name: Free disk space
+ run: dev/ci/util_free_space.sh
+
+ - name: Apply Gravitino MySQL schema
+ run: |
+ mysql -h 127.0.0.1 -P 3306 -ugravitino -pgravitino gravitino \
+ < scripts/mysql/schema-1.3.0-mysql.sql
+
+ - name: Build Gravitino distribution
+ run: ./gradlew compileDistribution -PskipWeb=true -x test
+
+ - name: Stage two instances (A=8090, B=8190)
+ run: |
+ set -euo pipefail
+ # MySQL JDBC driver: the entity store needs the connector in libs/,
not
+ # only under catalogs/jdbc-mysql/libs/ (which is for the JDBC catalog
+ # backend, not the entity store).
+ JDBC_JAR=distribution/package/libs/mysql-connector-java-8.0.14.jar
+ curl -fsSL \
+
https://repo1.maven.org/maven2/mysql/mysql-connector-java/8.0.14/mysql-connector-java-8.0.14.jar
\
+ -o "$JDBC_JAR"
+ # Verify integrity against Maven Central's published SHA-1.
+ EXPECTED_SHA1="$(curl -fsSL \
+
https://repo1.maven.org/maven2/mysql/mysql-connector-java/8.0.14/mysql-connector-java-8.0.14.jar.sha1)"
+ echo "${EXPECTED_SHA1} ${JDBC_JAR}" | sha1sum --check
Review Comment:
The MySQL Connector/J version downloaded here is 8.0.14, which is below the
repo’s documented recommended minimum (>= 8.0.16) and also inconsistent with
other repo tooling that uses newer 8.0.x drivers. Using <8.0.16 can trigger
compatibility warnings/limitations (see docs/jdbc-mysql-catalog.md) and may
cause unnecessary CI noise or subtle behavior differences.
##########
dev/ci/test_multi_instance_consistency.sh:
##########
@@ -0,0 +1,810 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#
+# Multi-instance consistency test for the JcasbinAuthorizer cache propagation.
+#
+# What this verifies (one cluster, two Gravitino instances backed by the same
DB):
+#
+# A. metadataIdCache / ownerRelCache are eventually consistent across
instances
+# via the JcasbinChangePoller (default poll interval = 3s).
+# → set owner on A, read it back on B.
+#
+# B. The new owner can actually exercise owner privileges on the *other*
instance
+# after the poll cycle, and the old owner is rejected.
+# → A sets owner=lisi; on B lisi can re-setOwner, zhangsan gets 403.
+#
+# C. userRoleCache (per-request, version-validated) reflects role grants
+# *immediately* on the other instance — no poll wait needed.
+# → grant role on A; on B a user with no prior privileges immediately
+# succeeds at an action gated by that role.
+#
+# D. Same path, reverse direction: revoke role on A is immediately effective
on B.
+#
+# E. loadedRoles cache (per-request, version-validated) reflects
role-privilege
+# changes immediately on the other instance.
+# → revoke MANAGE_USERS from role on A; user is denied on B even though
+# the role binding itself didn't change.
+# → grant CREATE_ROLE on the same role on A; user can immediately use it
on B.
+#
+# All authz-enforcement assertions exercise metalake-internal APIs (addUser,
+# createRole, setOwner) so the test runs without any external catalog backend.
+# The propagation guarantees being tested apply identically to schemas, tables,
+# filesets, etc. — only the path you'd PUT/GET differs.
+#
+# -----------------------------------------------------------------------------
+# Server preconditions (configure both instances BEFORE running):
+# gravitino.authorization.enable = true
+# gravitino.authenticators = simple
+# gravitino.authorization.serviceAdmins = admin #
required for metalake create
+# gravitino.entity-store = relational
+# gravitino.entity-store.relational.jdbcUrl = <shared
MySQL/PostgreSQL>
+# gravitino.authorization.jcasbin.changePollIntervalSecs = 3 (default;
lower = faster A↔B)
+#
+# H2 is NOT supported here — both instances must share one real SQL backend so
+# entity_change_log is visible to both pollers.
+#
+# Note on the "admin" role: per JcasbinAuthorizer.hasSetOwnerPermission,
service-admin
+# does NOT bypass ownership checks — only the current owner can setOwner. The
script
+# therefore does ownership chains as the *current* owner (admin → zhangsan →
lisi)
+# and uses a separate user (wangwu) for role-based tests so role grants and
+# ownership grants don't interfere.
+# -----------------------------------------------------------------------------
+#
+# Usage:
+# bash dev/ci/test_multi_instance_consistency.sh
+# INSTANCE_A=http://host1:8090 INSTANCE_B=http://host2:8090 bash
dev/ci/test_multi_instance_consistency.sh
+#
+# Exit code = number of failed assertions (0 = all pass).
+
+set -uo pipefail
+
+# ---- config ----------------------------------------------------------------
+
+INSTANCE_A="${INSTANCE_A:-http://localhost:8090}";
+INSTANCE_B="${INSTANCE_B:-http://localhost:8190}";
+ADMIN_USER="${ADMIN_USER:-admin}"
+POLL_WAIT_SECS="${POLL_WAIT_SECS:-6}" # default poll = 3s + buffer
+
+SUFFIX="$(date +%s)_$$"
+METALAKE="cons_test_${SUFFIX}"
+METALAKE2="cons_test2_${SUFFIX}" # for cross-metalake isolation check (Phase
J)
+USER_ZHANGSAN="zhangsan_${SUFFIX}"
+USER_LISI="lisi_${SUFFIX}"
+USER_WANGWU="wangwu_${SUFFIX}" # separate test user, never an owner
+USER_DELME="userdel_${SUFFIX}" # for delete-user / re-create tests (Phases
G, H)
+ROLE_NAME="role_${SUFFIX}"
+ROLE_FRESH="role_g_${SUFFIX}" # for Phase G (delete user test)
+ROLE_ALLOW="role_allow_${SUFFIX}" # for Phase K (DENY override)
+ROLE_DENY="role_deny_${SUFFIX}" # for Phase K
+GROUP_NAME="group_${SUFFIX}" # for Phase L
+TAG_NAME="tag_${SUFFIX}" # for Phase N
+
+PASS=0
+FAIL=0
+FAILED_TESTS=()
+
+# ---- helpers ---------------------------------------------------------------
+
+# Simple authenticator wants: Authorization: Basic base64(user:)
+auth_header() { printf 'Basic %s' "$(printf '%s:' "$1" | base64 | tr -d
'\n')"; }
+
+# api <base_url> <as_user> <method> <path> [body_json]
+# Writes status code to HTTP_CODE, body to RESPONSE_BODY.
+api() {
+ local base="$1" user="$2" method="$3" path="$4" body="${5:-}"
+ local tmp
+ tmp="$(mktemp -t graviton_api.XXXX)"
+ if [[ -n "$body" ]]; then
+ HTTP_CODE=$(curl -sS -o "$tmp" -w '%{http_code}' \
+ -H "Authorization: $(auth_header "$user")" \
+ -H 'Content-Type: application/json' \
+ -X "$method" --data "$body" "${base}${path}" || echo 000)
+ else
+ HTTP_CODE=$(curl -sS -o "$tmp" -w '%{http_code}' \
+ -H "Authorization: $(auth_header "$user")" \
+ -X "$method" "${base}${path}" || echo 000)
+ fi
Review Comment:
The curl calls in api() have no connect/overall timeout. If an instance
becomes unresponsive mid-run, the script can hang until the job-level timeout,
making CI failures slow and harder to diagnose. Add reasonable
--connect-timeout/--max-time limits to keep the test suite fail-fast.
--
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]
