-
Notifications
You must be signed in to change notification settings - Fork 375
made the regex more generic #3005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
dimas-b
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution, @cccs-cat001 ! In general the regex change looks reasonable to me. However, given that some existing applications may rely on the more strict regex (I do not know for sure), I've started a dev ML discussion too. Let's wait a few days for people to raise concerns (if any).
https://lists.apache.org/thread/dvgtn32h722h9xtvty84h21474q1b4jr
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java
Outdated
Show resolved
Hide resolved
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java
Show resolved
Hide resolved
runtime/service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java
Outdated
Show resolved
Hide resolved
|
|
||
| @ParameterizedTest | ||
| @ValueSource(strings = {"", "arn:aws:iam::0123456:role/jdoe", "aws-cn"}) | ||
| @ValueSource(strings = {"", "arn:aws:iam:0123456:role/jdoe", "aws-cn"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have another test for a valid non-aws role string which can pass the validation? So that, any regression will be caught if the regex change happens.
|
Thanks a lot for the change, @cccs-cat001 ! Thanks a lot for the review and dev mailing discussion, @dimas-b ! |
Following up on apache#3005, which allowed a wide range of ARN values in the validation RegEx, remove an additional explicit check for `aws-cn` being present in the ARN as a sub-string. Update existing unit tests to process `aws-cn` ARNs as common `aws` ARNs. Note: the old validation code does not look correct because it used to check for `aws-cn` anywhere in the ARN string, not just in its "partition" component.
Following up on #3005, which allowed a wide range of ARN values in the validation RegEx, remove an additional explicit check for `aws-cn` being present in the ARN as a sub-string. Update existing unit tests to process `aws-cn` ARNs as common `aws` ARNs. Note: the old validation code does not look correct because it used to check for `aws-cn` anywhere in the ARN string, not just in its "partition" component.
* Add loadEntities batch call and rename listFullEntities (apache#2508) * Add loadEntities batch call and rename listFullEntities * Changed batch call to implement loadResolvedEntities instead * Add loadResolvedEntities by id and entity cache support * Add additional test for loadResolvedEntities by id * Added additional test and updated comments in EntityCache interface * Add additional constructor to ResolvedEntitiesResult * Fixed unused method reference * Removed loadResolvedEntities method with lookup record param * Pulled out toResolvedPolarisEntity method per PR comment * Core: made the ARN role regex more generic (apache#3005) * fix(docs): Generify S3 index page (apache#2997) * Remove the mention of "cloud" since not all possible storage options are provided in "cloud". * Avoid listing specific child pages in the doc test. Rely on Hugo-general index (on the left-hand pane). --------- Co-authored-by: Alexandre Dutra <[email protected]> * fix(deps): update dependency io.prometheus:prometheus-metrics-exporter-servlet-jakarta to v1.4.3 (apache#3009) * fix(deps): update dependency com.google.cloud:google-cloud-storage-bom to v2.60.0 (apache#3011) * fix(deps): update dependency io.opentelemetry:opentelemetry-bom to v1.56.0 (apache#3012) * fix(deps): update dependency com.adobe.testing:s3mock-testcontainers to v4.10.0 (apache#3010) * fix(deps): update dependency org.agrona:agrona to v2.3.2 (apache#3014) * fix(deps): update quarkus platform and group to v3.29.2 (apache#3013) * chore(deps): update dependency pre-commit to v4.4.0 (apache#3015) * fix(deps): update dependency software.amazon.awssdk:bom to v2.38.2 (apache#3019) * Add test for TracingFilter (apache#2847) * NoSQL: Add (micro-ish) benchmarks (apache#3006) A project for JMH based benchmarks against NoSQL persistence. * Helm chart: include configmap checksum in deployment annotations (apache#3023) * fix(deps): update dependency ch.qos.logback:logback-classic to v1.5.21 (apache#3025) * NoSQL: Realms handling (apache#3007) Introduces handling for realms including realm-state management/transition. The `RealmStore` implementation for NoSQL depends on CDI components, coming in a follo-up PR. * Rename AccessConfig and AccessConfigProvider for clarity (apache#2883) * rename AccessConfig for clarity * rename getStorageAccessConfig() and add javadoc * Refactor: improve and clean up Dockerfiles (apache#2957) * Refactor: improve and clean up Dockerfiles * Refactor: improve and clean up Dockerfiles * Refactor: improve and clean up Dockerfiles * Refactor: improve and clean up Dockerfiles * Refactor: improve and clean up Dockerfiles * Refactor: improve and clean up Dockerfiles * Make StorageAccessConfigProvider request-scoped (apache#2974) - add `StorageCredentialsVendor` as request-scoped wrapper around `PolarisCredentialVendor` - make `FileIOFactory` request-scoped - make `TaskFileIOSupplier` request-scoped * Increase javadoc visibility in `nosql/realms` (apache#3029) This is to fix javadoc error: `No public or protected classes found to document` * NoSQL: Add correctness tests (apache#3027) Verifies the correctness of concurrent commits, and big index handling. These tests are intentionally _not_ part of the base-backend test suite for two reasons: 1. These tests do not run against the `Backend` interface but the `Persistence` interface, including commit and index logic. 2. These tests are intended to be runnable against a custom provisioned database cluster, not just tiny-ish test containers. * NoSQL: Add maintenance API, SPI (apache#3028) Maintenance operations include a bunch of tasks that are regularly executed against a backend database. Types of maintenance operations include: * Purging unreferenced objects and references within a catalog * Purging whole catalogs that are marked to be purged * Purging whole realms that are marked to be purged Implementation added in a follow-up PR. * Embrace request-scoped TokenBroker (apache#3024) * Embrace request-scoped TokenBroker `TokenBroker` and `CallContext` are both request-scoped, so instead of passing the former into the latter, we can do this via the `TokenBrokerFactory` and thus simplify the `TokenBroker` interface. * fix(deps): update dependency io.smallrye:jandex to v3.5.2 (apache#3032) * Fix monkey patching (apache#3016) * chore(deps): update quay.io/keycloak/keycloak docker tag to v26.4.5 (apache#3034) * chore(deps): update registry.access.redhat.com/ubi9/openjdk-21-runtime docker tag to v1.23-6.1762870925 (apache#3053) * fix(deps): update dependency org.testcontainers:testcontainers-bom to v2.0.2 (apache#3054) * chore(deps): update postgres docker tag to v18.1 (apache#3055) * Add Polaris Community Meeting 2025-11-13 (apache#3060) * Site: Rename menu "downloads" to "releases" (apache#2928) * Update dependency software.amazon.awssdk:bom to v2.38.7 (apache#3065) * Test-fix: Cleanup OPA test container on stop (apache#3041) Quarkus takes care of reusing a test-resource across tests. The current behavior leaves the container around. Plus some nit-fixes (deprecation + local var) * Update dependency org.apache.commons:commons-lang3 to v3.20.0 (apache#3063) * Build: ensure LICENSE/NOTICE is in all jars, always add pom-files to all jars (apache#3057) There are a some inconsistencies between the different kinds of jars and the included information: * LICENSE/NOTICE files are present in the "main" jar and in the sources jar, but not in the javadoc jar. * The Maven pom.xml and pom.properties files are only present for release builds or when explicitly requested. * "Additional" jar-manifest attributes that are only present in release builds. This change fixes the three mentioned issues: * Always include pom.xml and pom.properties in the built jar files. * Always include the additional jar-manifest attributes, except the Git information, which would otherwise render the Gradle build cache ineffective. * Include pom.xml + pom.properties + license/notice in literally all jar files. The Gradle logic to include the license+notice+pom files has been simplified as well. * Remove unused polarisEventListener field from IcebergCatalogHandler (apache#3045) it was added in c3f5001 but then its only usage was removed in d03c717 * fix(deps): update quarkus platform and group to v3.29.3 (apache#3052) * Site: Add Open Policy Agent (OPA) as External Policy Decision Point (apache#3030) Doc PR following up the introduction of OpaPolarisAuthorizer: apache#2680 * OPA: Tackle deprecation warnings (apache#3042) Instead of suppressing the deprecations, this change updates the code a little bit to remove the mocks (except to create a non-nullable parameter). * Use POJOs for OPA JSON schema construction and publish schema (apache#3031) Co-authored-by: Robert Stupp <[email protected]> * Use CDI for more test setups (apache#3040) this avoids a bunch of redundant manual setup. the important parts are establishing a `RealmContext` by calling `QuarkusMock.installMockForType` and then populating `polarisContext` from the injected `CallContext`. * fix(deps): update dependency com.github.dasniko:testcontainers-keycloak to v4 (apache#3070) * chore(deps): update actions/checkout digest to 93cb6ef (apache#3068) * OPA: Fail fast when OPA bearer token file is unreadable (apache#3062) * fix(deps): update immutables to v2.11.7 (apache#3072) * Skip Hugo Site workflow on forks (apache#3056) Forks usually don't have the "versioned-docs" tag and thus PRs against forks or rebasing the main branch on a fork currently always causes workflow failures. * Fix warnings around TransactionWorkspaceMetaStoreManager (apache#3044) - dont return `null` for interface methods that are `@Nonnull` - fix wrong method name parameters - dont annotate void methods as `@Nonnull` * NoSQL: Add CDI/common+testing + necessary nosql-store implementations (apache#3035) Adds common and test-specific CDI functionality. Requires the NoSQL store implementations `:polaris-persistence-nosql-realms-store-nosql` and `:polaris-nodes-store-nosql`. Those modules have cross-project dependencies for test purposes, hence those are all contained in this PR. CDI for Quarkus will be added in a follow-up. * Automate the release guide - Take 2 - Github workflows (apache#2383) The release automation is simplified to four GitHub workflows that just require the really mandatory user input: the version number. 1. workflow: Trigger the creation of the release branch 2. workflow: Upgrade the release branch with the version and build the the final change-log for that version 3. workflow: Build the RC artifacts from the release branch and push those to the various staging repositories 4. workflow: Eventually release the artifacts. See also the [email announcement](https://lists.apache.org/thread/d0smz07gnr509yj5dc6omo3cvkf1pnh7). --------- Co-authored-by: Robert Stupp <[email protected]> * Update actions/checkout digest to 93cb6ef (apache#3082) * NoSQL: adapt to conflicting changes in main * Last merged commit 8ccddc5 --------- Co-authored-by: Michael Collado <[email protected]> Co-authored-by: cccs-cat001 <[email protected]> Co-authored-by: Dmitri Bourlatchkov <[email protected]> Co-authored-by: Alexandre Dutra <[email protected]> Co-authored-by: Mend Renovate <[email protected]> Co-authored-by: Nuoya Jiang <[email protected]> Co-authored-by: Yong Zheng <[email protected]> Co-authored-by: Christopher Lambert <[email protected]> Co-authored-by: JB Onofré <[email protected]> Co-authored-by: Yufei Gu <[email protected]> Co-authored-by: Sung Yun <[email protected]> Co-authored-by: Pierre Laporte <[email protected]>
* Do not fail a release when markdown-link-check check fails as it is flaky (apache#3116) * Source tarball reproducible (apache#3143) `git --mtime` MUST use the time zone for reproducible builds. * Skip release e-mail templates from svn dist copy (apache#3147) * Make pom.xml always reproducible (apache#3145) It turned out in practice, that there's no guarantee that the `<parent>` element in `pom.xml` files always appear at the same place. This change ensures that the `<parent>` elements always appears at a deterministic location at the top of `pom.xml` files. * Fix executable POSIX permission in archive files (apache#3146) The PR apache#2819 accidentally _removed_ the executable POSIX file permission, assuming that not explicity setting the attributes via `filePermissions` retains the file-system 'x' permission. This change updates the logic to explicitly check the owner-executable bit and uses `755` or `644` respectively for each individual file in the archive. * Spark: Initial integration for hudi tables within Polaris (apache#1862) * Update actions/setup-python digest to 83679a8 (apache#3157) * Update actions/stale digest to 5611b9d (apache#3155) * Fix LICENSE and NOTICE in the distributions and docker images. (apache#3125) * Remove readEntity() call (apache#3111) Calling readEntity() is not allowed server-side by some HTTP servers. * Run CI on release branches (apache#3121) The release workflows check whether CI passes for the required checks. This would fail, because CI isn't configured to run on release branches. This change lets CI run on `release/*` branches. * adding support to use a kms key for s3 buckets data encryption (AWS only) (apache#2802) Add catalog-level support for KMS with s3 buckets * Update plugin jetbrains-changelog to v2.5.0 (apache#3166) * Update quay.io/keycloak/keycloak Docker tag to v26.4.6 (apache#3163) * NoSQL: Prepare admin-tool (apache#3134) No functional changes. 1. Refactor the configuration property to a configuration type. 2. Make `BaseCommand` suitable for non-meta-store-factory use cases. * Iceberg-Catalog: also set catalog-id for location overlap checks (apache#3136) * Fix catalog-role creating in `PolarisTestMetaStoreManager` (apache#3122) `testLookup()` attempts to check for a catalog-role on catalog ID 0, which is an illegal ID for a catalog. Fix is to move the assertion below the catalog creation. * Releasy: prepare for Helm 4 (helm package repro) (apache#3088) Part of apache#3086 * Update Quarkus Platform and Group to v3.30.1 (apache#3168) * Relax ARN validation logic (apache#3071) Following up on apache#3005, which allowed a wide range of ARN values in the validation RegEx, remove an additional explicit check for `aws-cn` being present in the ARN as a sub-string. Update existing unit tests to process `aws-cn` ARNs as common `aws` ARNs. Note: the old validation code does not look correct because it used to check for `aws-cn` anywhere in the ARN string, not just in its "partition" component. * docs: Add François as Mentor (apache#3162) * docs: Add François as Mentor * update mentor list according to ASF project info * Event type IDs + event metadata incl. OTel context (apache#2998) This PR implements the action items from the following discussion threads: - https://lists.apache.org/thread/yx7pkgczl6k7bt4k4yzqrrq9gn7gqk2p - https://lists.apache.org/thread/rl5cpcft16sn5n00mfkmx9ldn3gsqtfy - https://lists.apache.org/thread/5dpyo0nn2jbnjtkgv0rm1dz8mpt132j9 Summary of changes: - Introduced a `PolarisEventType` enum holding the 150+ event types. - Introduced a `PolarisEventMetadata` interface as suggested by @adnanhemani, exposing: event ID, timestamp, realm ID, principal, request ID, and OTel context. - Introduced a `PolarisEventMetadataFactory` to centralize the logic for gathering the various elements of an event metadata. - Modified `PolarisEvent` to expose 3 new methods: - `PolarisEventType type()` - `PolarisEventMetadata metadata()` - Persistence of OTel context is done in `additional_properties` as suggested by @flyrain. - Added `InMemoryBufferEventListenerIntegrationTest` to verify that all contextual data is properly persisted. * fix typo in management API yaml (apache#3172) * Fix homepage Get Started button layout (apache#3169) Wrap the Get Started button in a div container to prevent it from becoming inline with text at certain screen widths. Follows Docsy blocks/cover shortcode pattern. * fix OPA javadoc referencing `OpaSchemaGenerator` (apache#3153) `OpaSchemaGenerator` is not on the classpath of `opa/impl/main` so the javadoc tool is not able to resolve a `@link` to it. Use `@code` instead to avoid build warnings like the following: * Update dependency com.azure:azure-sdk-bom to v1.3.3 (apache#3179) * Update dependency com.google.errorprone:error_prone_core to v2.45.0 (apache#3177) * test: Add Some Spark Client Tests and Update Documentation on Generic Tables (apache#3152) * Site: Make homepage image full-width (apache#3171) Add CSS class to allow images to span full viewport width by canceling out container padding. Apply to homepage hero image using AsciiDoc role attribute. * chore(enhancement): gitignore application-local.properties (apache#3175) * Update registry.access.redhat.com/ubi9/openjdk-21-runtime Docker tag to v1.23-6.1764155306 (apache#3186) * Update quay.io/keycloak/keycloak Docker tag to v26.4.7 (apache#3185) * Update dependency software.amazon.awssdk:bom to v2.39.6 (apache#3184) * Testing: increase visibility + make PCC/PMSM accessible (apache#3137) * `BasePolarisMetaStoreManagerTest`: make `PolarisCallContext` + `PolarisMetaStoreManager` + `PolarisTestMetaStoreManager` accessible by subclasses * Make constants of `PolarisRestCatalogMinIOIT` accessible * Update docker.io/prom/prometheus Docker tag to v3.8.0 (apache#3191) * Update helm/chart-testing-action action to v2.8.0 (apache#2982) * chore(enhancement): make custom hidden tasks visible in ./gradlew tasks (apache#3176) * fix type cast warning in PolarisCatalogUtils (apache#3178) ``` plugins/spark/v3.5/spark/src/main/java/org/apache/polaris/spark/utils/PolarisCatalogUtils.java:131: warning: [unchecked] unchecked cast scala.collection.immutable.Map$.MODULE$.apply( ^ required: Map<String,String> found: Map ``` * chore(deps): update actions/stale digest to 9971854 (apache#3197) * fix(deps): update dependency io.smallrye:jandex to v3.5.3 (apache#3193) * chore(deps): update actions/checkout digest to 8e8c483 (apache#3192) * added venv to the gitignore (apache#3199) * CLI: Add Hive federation option (apache#2798) * chore(deps): update docker.io/jaegertracing/all-in-one docker tag to v1.76.0 (apache#3201) * chore(deps): update registry.access.redhat.com/ubi9/openjdk-21-runtime docker tag to v1.23-6.1764562148 (apache#3202) * fix(deps): update quarkus platform and group to v3.30.2 (apache#3198) * chore(deps): update dependency boto3 to ~=1.42.2 (apache#3126) * NoSQL: CDI / Quarkus (apache#3135) * fix(deps): update dependency com.adobe.testing:s3mock-testcontainers to v4.11.0 (apache#3208) * Update dependency mypy to >=1.19, <=1.19.0 (apache#3180) * chore(deps): update actions/setup-java digest to f2beeb2 (apache#3206) * Fix spelling in comments (apache#3212) * Make each task attempt run in a dedicated CDI request context (apache#3210) * Make each task attempt run in a dedicated CDI request context Currently, tasks inherit the CDI context from the requests that submitted them, but run asynchronously. Therefore, if the original request context ends, the task may not be able to use the expired beans for that context. This change makes each task run in its own dedicated CDI request context with `RealmContext` explicitly propagated in `TaskExecutorImpl`. Test-only error handlers are added to `TaskExecutorImpl` to facilitate detecting task errors during CI. Fixes apache#3203 * fix(deps): update dependency com.gradleup.shadow:shadow-gradle-plugin to v9.3.0 (apache#3218) * Last merged commit be3c88b --------- Co-authored-by: Pierre Laporte <[email protected]> Co-authored-by: Rahil C <[email protected]> Co-authored-by: Mend Renovate <[email protected]> Co-authored-by: JB Onofré <[email protected]> Co-authored-by: Alexandre Dutra <[email protected]> Co-authored-by: fabio-rizzo-01 <[email protected]> Co-authored-by: Dmitri Bourlatchkov <[email protected]> Co-authored-by: Tamas Mate <[email protected]> Co-authored-by: Adam Christian <[email protected]> Co-authored-by: Artur Rakhmatulin <[email protected]> Co-authored-by: cccs-cat001 <[email protected]> Co-authored-by: Yufei Gu <[email protected]> Co-authored-by: Yong Zheng <[email protected]>
We found a use case where the AWS ARN regex pattern was too specific and caused us issues connecting to our private S3 storage that's not on AWS. This fixes that issue.
Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)