Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add test for security annotations in rest-data-panache #994

Merged
merged 1 commit into from
Jan 30, 2023
Merged

Add test for security annotations in rest-data-panache #994

merged 1 commit into from
Jan 30, 2023

Conversation

jsmrcka
Copy link
Contributor

@jsmrcka jsmrcka commented Jan 13, 2023 •
edited
Loading

Summary

Quarkus extensions based on rest-data-panache support propagation of security annotations into generated JAX-RS resources. These tests provide coverage of this feature for extensions:

  • quarkus-hibernate-orm-rest-data-panache
  • quarkus-spring-data-rest

See also related test plan:

Please select the relevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • Dependency update
  • Refactoring
  • Backport
  • New scenario (non-breaking change which adds functionality)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

@jsmrcka jsmrcka added the triage/backport-2.13? It needs to backport changes to branch 2.13 label Jan 13, 2023
@jsmrcka jsmrcka requested a review from rsvoboda January 13, 2023 14:56
@jsmrcka jsmrcka self-assigned this Jan 13, 2023
@jsmrcka jsmrcka force-pushed the feature/rest-data-panache-security-annotations branch from ac96b8b to 548e7d2 Compare January 13, 2023 15:21
@rsvoboda
Copy link
Member

run tests

@rsvoboda
Copy link
Member

@jsmrcka investigate / fix the failures pls

@jsmrcka jsmrcka force-pushed the feature/rest-data-panache-security-annotations branch from 548e7d2 to 6f816de Compare January 17, 2023 11:13
@rsvoboda
Copy link
Member

rsvoboda commented Jan 17, 2023 •
edited
Loading

[INFO] Quarkus QE TS: SQL Database: Panache + Flyway ...... FAILURE [08:55 min]

Caused by: org.graalvm.compiler.java.BytecodeParser$BytecodeParserError: com.oracle.graal.pointsto.constraints.UnresolvedElementException: Discovered unresolved method during parsing: io.quarkus.ts.sqldb.panacheflyway.secured.EntityPermitAllResourceJaxRs_85bee2292b5586a6a98e6acaec55160090342fc2.count(). This error is reported at image build time because class io.quarkus.ts.sqldb.panacheflyway.secured.EntityPermitAllResourceJaxRs_85bee2292b5586a6a98e6acaec55160090342fc2_Subclass is registered for linking at image build time by command line

Native image build fails.

@rsvoboda
Copy link
Member

rsvoboda commented Jan 18, 2023 •
edited
Loading

The failures in native are caused by quarkus-elytron-security-properties-file dependency

Compilation into native image works fine when I comment out the dependency - mvn -f sql-db/panache-flyway clean package -Dnative -DskipTests -DskipITs

@jsmrcka please file the GH issue in the Quarkus repo, CC @Sgitario

Native image build fail is imho related to the dynamically generate methods available in RestDataResource interface - e.g. count().

Caused by: com.oracle.graal.pointsto.constraints.UnresolvedElementException: Discovered unresolved method during parsing: io.quarkus.ts.sqldb.panacheflyway.secured.EntityPermitAllResourceJaxRs_85bee2292b5586a6a98e6acaec55160090342fc2.count(). This error is reported at image build time because class io.quarkus.ts.sqldb.panacheflyway.secured.EntityPermitAllResourceJaxRs_85bee2292b5586a6a98e6acaec55160090342fc2_Subclass is registered for linking at image build time by command line

@rsvoboda
Copy link
Member

Another interesting fact is that spring-data module receive similar changes but the compilation into native works fine.

@jsmrcka jsmrcka force-pushed the feature/rest-data-panache-security-annotations branch from 6f816de to 16687bf Compare January 21, 2023 14:17
@jsmrcka
Copy link
Contributor Author

jsmrcka commented Jan 21, 2023

Native failure reported:

Native build of the sql-db/panache-flyway module disabled:

  • 16687bf
  • Do not backport this commit to 2.13 - the issue does not affect Quarkus 2.13.

@mjurc mjurc requested review from pjgg and removed request for rsvoboda January 23, 2023 08:44
pjgg
pjgg suggested changes Jan 23, 2023
View reviewed changes
Copy link
Contributor

@pjgg pjgg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, check CI checkstyle validation errors!

...a/src/test/java/io/quarkus/ts/spring/data/rest/secured/SecuredRepositoryRestResourcesIT.java Outdated Show resolved Hide resolved
sql-db/panache-flyway/pom.xml Outdated Show resolved Hide resolved
@jsmrcka
Copy link
Contributor Author

jsmrcka commented Jan 23, 2023 •
edited
Loading

Also, check CI checkstyle validation errors!

Reported here: quarkusio/quarkus#30540.

EDIT: fixed.

@jsmrcka jsmrcka mentioned this pull request Jan 24, 2023
Merged
@jsmrcka
Add test for security annotations in rest-data-panache
fc9a4b6 
Quarkus extensions based on `rest-data-panache` support propagation of
security annotations into generated JAX-RS resources.
These tests provide coverage of this feature for extensions:
- `quarkus-hibernate-orm-rest-data-panache`
- `quarkus-spring-data-rest`

See also related test plan:
- https://github.com/quarkus-qe/quarkus-test-plans/blob/main/QUARKUS-2788.md
@jsmrcka jsmrcka force-pushed the feature/rest-data-panache-security-annotations branch from 16687bf to fc9a4b6 Compare January 24, 2023 12:27
@jsmrcka
Copy link
Contributor Author

jsmrcka commented Jan 25, 2023

run tests

1 similar comment
@jsmrcka
Copy link
Contributor Author

jsmrcka commented Jan 25, 2023

run tests

@jsmrcka jsmrcka requested a review from pjgg January 25, 2023 16:32
pjgg
pjgg approved these changes Jan 29, 2023
View reviewed changes
Copy link
Contributor

@pjgg pjgg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but let's hold this PR until having a look OCP results

@pjgg pjgg merged commit 3f7a4c8 into quarkus-qe:main Jan 30, 2023
pjgg pushed a commit to pjgg/quarkus-test-suite that referenced this pull request Jan 30, 2023
@jsmrcka
Add test for security annotations in rest-data-panache (quarkus-qe#994)
cc476cd 
Quarkus extensions based on `rest-data-panache` support propagation of
security annotations into generated JAX-RS resources.
These tests provide coverage of this feature for extensions:
- `quarkus-hibernate-orm-rest-data-panache`
- `quarkus-spring-data-rest`

See also related test plan:
- https://github.com/quarkus-qe/quarkus-test-plans/blob/main/QUARKUS-2788.md

(cherry picked from commit 3f7a4c8)
@pjgg pjgg mentioned this pull request Jan 30, 2023
Merged
1 task
pjgg pushed a commit to pjgg/quarkus-test-suite that referenced this pull request Jan 30, 2023
@jsmrcka
Add test for security annotations in rest-data-panache (quarkus-qe#994)
07b4daa 
Quarkus extensions based on `rest-data-panache` support propagation of
security annotations into generated JAX-RS resources.
These tests provide coverage of this feature for extensions:
- `quarkus-hibernate-orm-rest-data-panache`
- `quarkus-spring-data-rest`

See also related test plan:
- https://github.com/quarkus-qe/quarkus-test-plans/blob/main/QUARKUS-2788.md

(cherry picked from commit 3f7a4c8)
@jsmrcka jsmrcka deleted the feature/rest-data-panache-security-annotations branch February 1, 2023 07:19
pjgg pushed a commit that referenced this pull request Feb 1, 2023
@fedinskiy @rsvoboda @jsmrcka
[2.13] Backport several scenarios related to HTTP, SQL and Monitoring…
fa5a391 
… modules (#1021)

* Test Extended Architecture (XA) connection

Provides coverage for https://issues.redhat.com/browse/QUARKUS-2742

(cherry picked from commit ce15b26)

* Improve reactive rest client "process paths before sub resources" scenario (#995)

There is an issue in upstream (quarkusio/quarkus#29821) that
only happens under some RestClient definition hierarchy.

This commit reproduces the problem in Quarkus 2.12 and earlier versions

(cherry picked from commit 2350b46)

* New Scenario: RequestScope custom context was removed after `javax.enterprise` event propagation

When firing an async CDI Event, the requestScope context from the emitter briefly exists for the observer
and is then terminated. This commit is a reproducer of this scenario that happens on Quarkus 2.13.0.Final

(cherry picked from commit ef3eed6)

* gRPC and SSE coverage for OpenTelemetry

(cherry picked from commit f327c60)

* Add coverage to eventbus '@ConsumeEvent' annotation

(cherry picked from commit 7da0d4b)

* Drop duplicated definition of quarkus-opentelemetry

(cherry picked from commit b0cba7f)

* OutboundSseEvent is not correctly serialized

(cherry picked from commit 99b5eb1)

* Check, that dev-mode is omitted on projects with pom packaging

Required for QUARKUS-2757

(cherry picked from commit 4fd38c7)

* Refactoring of QUARKUS-2748: adding http test and improving error messages

(cherry picked from commit 6563431)

* Add test for security annotations in rest-data-panache (#994)

Quarkus extensions based on `rest-data-panache` support propagation of
security annotations into generated JAX-RS resources.
These tests provide coverage of this feature for extensions:
- `quarkus-hibernate-orm-rest-data-panache`
- `quarkus-spring-data-rest`

See also related test plan:
- https://github.com/quarkus-qe/quarkus-test-plans/blob/main/QUARKUS-2788.md

(cherry picked from commit 3f7a4c8)

* Add transaction-API classic scenario

(cherry picked from commit 509d491)

* Cover Vert.X-specific metrics. (#1019)

Add new module, which uses Vert.X-based HTTP server
Verify, that it works, and that it creates all required metrics
Required for https://issues.redhat.com/browse/QUARKUS-2829

(cherry picked from commit 818837f)

* Add missing opentelemetry exporter to Narayana scenario

---------

Co-authored-by: Fedor Dudinskiy <[email protected]>
Co-authored-by: Rostislav Svoboda <[email protected]>
Co-authored-by: Josef Smrcka <[email protected]>
@pjgg pjgg added this to the 2.7 milestone Feb 2, 2023
@pjgg pjgg removed the triage/backport-2.13? It needs to backport changes to branch 2.13 label Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pjgg pjgg pjgg approved these changes

Assignees

@jsmrcka jsmrcka

Labels
None yet
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.
3 participants
@jsmrcka @rsvoboda @pjgg