[GitHub] [camel-quarkus] ppalaga opened a new pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga opened a new pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
ppalaga opened a new pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647
 
 
   reflection
   
   Removal of NettyConfiguration, NettyServerBootstrapConfiguration, PdfConfiguration registration led to native failures so these need a separate investigation.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
lburgazzoli commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#issuecomment-576817473
 
 
   ok to test

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#issuecomment-577044836
 
 
   Strange, the Consul test is passing for me locally. Let me figure out what is different here.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#issuecomment-577045850
 
 
   A race for the configurers set on the component is my first theory.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#issuecomment-577131556
 
 
   > A race for the configurers set on the component is my first theory.
   
   Much simpler: stale deployment modules in my local Maven repo
   
   Jenkins was right and I can reproduce.
   
   Looks like this change needs to wait for Camel 3.1.0 because https://issues.apache.org/jira/browse/CAMEL-14263 was not fixed for all components in 3.0.1.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#issuecomment-577621881
 
 
   36102f2 rebased and tested properly

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370073728
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
 
 Review comment:
   would' be better to have a build item generated by the respective extensions ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370073728
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
 
 Review comment:
   would' be better to have a build item generated by the respective extensions ? developer writing custom camel component extension outside this repo could also leverage that build item if needed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370086006
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
 
 Review comment:
   No, I'd prefer a central location to see better what is whitelisted. I hope the list will gradually disappear.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370086411
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
 
 Review comment:
   But external extension won't have a way to make it working

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370090912
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
+
+                /* org.apache.camel.component.consul.* can be removed after the upgrade to Camel 3.1 */
+                "org.apache.camel.component.consul.ConsulConfiguration",
+                "org.apache.camel.component.consul.ConsulClientConfiguration",
+                "org.apache.camel.component.consul.health.ConsulHealthCheckRepositoryConfiguration",
+                "org.apache.camel.component.consul.cloud.ConsulServiceRegistryConfiguration"));
+
+        @BuildStep
+        void bannedReflectiveClasses(
+                CombinedIndexBuildItem combinedIndex,
+                List<ReflectiveClassBuildItem> reflectiveClass,
+                BuildProducer<GeneratedResourceBuildItem> dummy // to force the execution of this method
+        ) {
+            final DotName uriParamsDotName = DotName.createSimple("org.apache.camel.spi.UriParams");
+
+            final Set<String> bannedClassNames = combinedIndex.getIndex()
+                    .getAnnotations(uriParamsDotName)
+                    .stream()
+                    .filter(ai -> ai.target().kind() == Kind.CLASS)
+                    .map(ai -> ai.target().asClass().name().toString())
+                    .collect(Collectors.toSet());
+
+            Set<String> violations = reflectiveClass.stream()
+                    .map(ReflectiveClassBuildItem::getClassNames)
+                    .flatMap(Collection::stream)
+                    .filter(cl -> !URI_PARAMS_WHITELIST.contains(cl))
+                    .filter(bannedClassNames::contains)
+                    .collect(Collectors.toSet());
+
+            if (!violations.isEmpty()) {
+                throw new IllegalStateException(
 
 Review comment:
   all the camel components have a property that can control if the generated configurer have to be used or not so I feel we should log a warning but I'd rather avoid to fail the build.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370090912
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
+
+                /* org.apache.camel.component.consul.* can be removed after the upgrade to Camel 3.1 */
+                "org.apache.camel.component.consul.ConsulConfiguration",
+                "org.apache.camel.component.consul.ConsulClientConfiguration",
+                "org.apache.camel.component.consul.health.ConsulHealthCheckRepositoryConfiguration",
+                "org.apache.camel.component.consul.cloud.ConsulServiceRegistryConfiguration"));
+
+        @BuildStep
+        void bannedReflectiveClasses(
+                CombinedIndexBuildItem combinedIndex,
+                List<ReflectiveClassBuildItem> reflectiveClass,
+                BuildProducer<GeneratedResourceBuildItem> dummy // to force the execution of this method
+        ) {
+            final DotName uriParamsDotName = DotName.createSimple("org.apache.camel.spi.UriParams");
+
+            final Set<String> bannedClassNames = combinedIndex.getIndex()
+                    .getAnnotations(uriParamsDotName)
+                    .stream()
+                    .filter(ai -> ai.target().kind() == Kind.CLASS)
+                    .map(ai -> ai.target().asClass().name().toString())
+                    .collect(Collectors.toSet());
+
+            Set<String> violations = reflectiveClass.stream()
+                    .map(ReflectiveClassBuildItem::getClassNames)
+                    .flatMap(Collection::stream)
+                    .filter(cl -> !URI_PARAMS_WHITELIST.contains(cl))
+                    .filter(bannedClassNames::contains)
+                    .collect(Collectors.toSet());
+
+            if (!violations.isEmpty()) {
+                throw new IllegalStateException(
 
 Review comment:
   all the camel components have a property that can control if the generated configurer have to be used or not so as lonng as the property is there, I feel we should log a warning but I'd rather avoid to fail the build.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370090912
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
+
+                /* org.apache.camel.component.consul.* can be removed after the upgrade to Camel 3.1 */
+                "org.apache.camel.component.consul.ConsulConfiguration",
+                "org.apache.camel.component.consul.ConsulClientConfiguration",
+                "org.apache.camel.component.consul.health.ConsulHealthCheckRepositoryConfiguration",
+                "org.apache.camel.component.consul.cloud.ConsulServiceRegistryConfiguration"));
+
+        @BuildStep
+        void bannedReflectiveClasses(
+                CombinedIndexBuildItem combinedIndex,
+                List<ReflectiveClassBuildItem> reflectiveClass,
+                BuildProducer<GeneratedResourceBuildItem> dummy // to force the execution of this method
+        ) {
+            final DotName uriParamsDotName = DotName.createSimple("org.apache.camel.spi.UriParams");
+
+            final Set<String> bannedClassNames = combinedIndex.getIndex()
+                    .getAnnotations(uriParamsDotName)
+                    .stream()
+                    .filter(ai -> ai.target().kind() == Kind.CLASS)
+                    .map(ai -> ai.target().asClass().name().toString())
+                    .collect(Collectors.toSet());
+
+            Set<String> violations = reflectiveClass.stream()
+                    .map(ReflectiveClassBuildItem::getClassNames)
+                    .flatMap(Collection::stream)
+                    .filter(cl -> !URI_PARAMS_WHITELIST.contains(cl))
+                    .filter(bannedClassNames::contains)
+                    .collect(Collectors.toSet());
+
+            if (!violations.isEmpty()) {
+                throw new IllegalStateException(
 
 Review comment:
   all the camel components have a property that can control if the generated configurer have to be used or not so as long as the property is there, I feel we should log a warning but I'd rather avoid to fail the build.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370101239
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
 
 Review comment:
   True. I was thinking of creating a dedicated extension for this (let's call it "policy extension") that would only be pulled by our tests. This would allow us to enforce the policy for our extensions and let 3rd party extensions do what they want. WDYT?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370110506
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
 
 Review comment:
   I really do not like to have to touch core to make an extension working so if it were me to decide, I'd go for a simple build item also, I do not know it it really make sense to put such enforcement

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370116321
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
 ##########
 @@ -68,6 +71,66 @@
                 PropertiesComponent.class,
                 DataFormat.class);
 
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        private static final Set<String> URI_PARAMS_WHITELIST = new HashSet<>(Arrays.asList(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
 
 Review comment:
   > I really do not like to have to touch core to make an extension working so if it were me to decide, I'd go for a simple build item.
   
   I still think policy definitions are better to keep centrally, but OK, let me try to implement this with a build item.
   
   > also, I do not know it it really make sense to put such enforcement
   
   Well, I think this is the most effective, scalable (and sustainable) way to reduce the use of reflection. I do not think it is possible to be able to keep control once we will have 100+ extensions.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
ppalaga commented on issue #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#issuecomment-577722396
 
 
   c4c1663 introduces a build item as requested by @lburgazzoli

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370191753
 
 

 ##########
 File path: extensions/support/policy/deployment/src/main/java/org/apache/camel/quarkus/component/support/policy/deployment/PolicyProcessor.java
 ##########
 @@ -0,0 +1,112 @@
+/*
+ * 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.
+ */
+package org.apache.camel.quarkus.component.support.policy.deployment;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import io.quarkus.deployment.annotations.BuildProducer;
+import io.quarkus.deployment.annotations.BuildStep;
+import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
+import io.quarkus.deployment.builditem.FeatureBuildItem;
+import io.quarkus.deployment.builditem.GeneratedResourceBuildItem;
+import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
+import org.apache.camel.quarkus.core.deployment.UnbannedReflectiveBuildItem;
+import org.jboss.jandex.AnnotationTarget.Kind;
+import org.jboss.jandex.DotName;
+
+class PolicyProcessor {
+
+    private static final String FEATURE = "camel-policy";
+
+    @BuildStep
+    FeatureBuildItem feature() {
+        return new FeatureBuildItem(FEATURE);
+    }
+
+    @BuildStep
+    UnbannedReflectiveBuildItem unbanReflectives() {
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        return new UnbannedReflectiveBuildItem(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
+
+                /* org.apache.camel.component.consul.* can be removed after the upgrade to Camel 3.1 */
+                "org.apache.camel.component.consul.ConsulConfiguration",
+                "org.apache.camel.component.consul.ConsulClientConfiguration",
+                "org.apache.camel.component.consul.health.ConsulHealthCheckRepositoryConfiguration",
+                "org.apache.camel.component.consul.cloud.ConsulServiceRegistryConfiguration");
+    }
+
+    /* Make the build fail as long as there are banned classes registered for reflection */
+    @BuildStep
+    void bannedReflectiveClasses(
+            CombinedIndexBuildItem combinedIndex,
+            List<ReflectiveClassBuildItem> reflectiveClasses,
+            List<UnbannedReflectiveBuildItem> unbannedReflectives,
+            BuildProducer<GeneratedResourceBuildItem> dummy // to force the execution of this method
+    ) {
+        final DotName uriParamsDotName = DotName.createSimple("org.apache.camel.spi.UriParams");
+
+        final Set<String> bannedClassNames = combinedIndex.getIndex()
+                .getAnnotations(uriParamsDotName)
+                .stream()
+                .filter(ai -> ai.target().kind() == Kind.CLASS)
+                .map(ai -> ai.target().asClass().name().toString())
+                .collect(Collectors.toSet());
+
+        final Set<String> unbannedClassNames = unbannedReflectives.stream()
+                .map(UnbannedReflectiveBuildItem::getClassNames)
+                .flatMap(Collection::stream)
+                .collect(Collectors.toSet());
+
+        Set<String> violations = reflectiveClasses.stream()
+                .map(ReflectiveClassBuildItem::getClassNames)
+                .flatMap(Collection::stream)
+                .filter(cl -> !unbannedClassNames.contains(cl))
+                .filter(bannedClassNames::contains)
+                .collect(Collectors.toSet());
+
+        if (!violations.isEmpty()) {
+            throw new IllegalStateException(
 
 Review comment:
   Should we really fail the build ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370193373
 
 

 ##########
 File path: extensions/support/policy/deployment/src/main/java/org/apache/camel/quarkus/component/support/policy/deployment/PolicyProcessor.java
 ##########
 @@ -0,0 +1,112 @@
+/*
+ * 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.
+ */
+package org.apache.camel.quarkus.component.support.policy.deployment;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import io.quarkus.deployment.annotations.BuildProducer;
+import io.quarkus.deployment.annotations.BuildStep;
+import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
+import io.quarkus.deployment.builditem.FeatureBuildItem;
+import io.quarkus.deployment.builditem.GeneratedResourceBuildItem;
+import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
+import org.apache.camel.quarkus.core.deployment.UnbannedReflectiveBuildItem;
+import org.jboss.jandex.AnnotationTarget.Kind;
+import org.jboss.jandex.DotName;
+
+class PolicyProcessor {
+
+    private static final String FEATURE = "camel-policy";
+
+    @BuildStep
+    FeatureBuildItem feature() {
+        return new FeatureBuildItem(FEATURE);
+    }
+
+    @BuildStep
+    UnbannedReflectiveBuildItem unbanReflectives() {
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        return new UnbannedReflectiveBuildItem(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
+
+                /* org.apache.camel.component.consul.* can be removed after the upgrade to Camel 3.1 */
+                "org.apache.camel.component.consul.ConsulConfiguration",
+                "org.apache.camel.component.consul.ConsulClientConfiguration",
+                "org.apache.camel.component.consul.health.ConsulHealthCheckRepositoryConfiguration",
+                "org.apache.camel.component.consul.cloud.ConsulServiceRegistryConfiguration");
 
 Review comment:
   It does not change much from the previous implementation as when we'll remove i.e. `PdfConfiguration` from the pdf extension we have to remember to get to this artefact and change it.
   
   I really don't get why this should not be side-by-side with the code declaring the need for the reflective classes.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #647: Fix #518 Rely on configurers for Configuration classes instead of using
URL: https://github.com/apache/camel-quarkus/pull/647#discussion_r370202236
 
 

 ##########
 File path: extensions/support/policy/deployment/src/main/java/org/apache/camel/quarkus/component/support/policy/deployment/PolicyProcessor.java
 ##########
 @@ -0,0 +1,112 @@
+/*
+ * 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.
+ */
+package org.apache.camel.quarkus.component.support.policy.deployment;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import io.quarkus.deployment.annotations.BuildProducer;
+import io.quarkus.deployment.annotations.BuildStep;
+import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
+import io.quarkus.deployment.builditem.FeatureBuildItem;
+import io.quarkus.deployment.builditem.GeneratedResourceBuildItem;
+import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
+import org.apache.camel.quarkus.core.deployment.UnbannedReflectiveBuildItem;
+import org.jboss.jandex.AnnotationTarget.Kind;
+import org.jboss.jandex.DotName;
+
+class PolicyProcessor {
+
+    private static final String FEATURE = "camel-policy";
+
+    @BuildStep
+    FeatureBuildItem feature() {
+        return new FeatureBuildItem(FEATURE);
+    }
+
+    @BuildStep
+    UnbannedReflectiveBuildItem unbanReflectives() {
+        /**
+         * A list of classes annotated with <code>@UriParams</code> which we accept to be registered for reflection
+         * mostly because there are errors when they are removed. TODO: solve the underlying problems and remove as
+         * many entries as possible from the list.
+         */
+        return new UnbannedReflectiveBuildItem(
+                "org.apache.camel.support.processor.DefaultExchangeFormatter",
+                "org.apache.camel.component.pdf.PdfConfiguration",
+                "org.apache.camel.component.netty.NettyConfiguration",
+                "org.apache.camel.component.netty.NettyServerBootstrapConfiguration",
+                "org.apache.camel.component.fhir.FhirUpdateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirOperationEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirConfiguration",
+                "org.apache.camel.component.fhir.FhirLoadPageEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirSearchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirTransactionEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCreateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirValidateEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirReadEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirCapabilitiesEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirHistoryEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirMetaEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirPatchEndpointConfiguration",
+                "org.apache.camel.component.fhir.FhirDeleteEndpointConfiguration",
+
+                /* org.apache.camel.component.consul.* can be removed after the upgrade to Camel 3.1 */
+                "org.apache.camel.component.consul.ConsulConfiguration",
+                "org.apache.camel.component.consul.ConsulClientConfiguration",
+                "org.apache.camel.component.consul.health.ConsulHealthCheckRepositoryConfiguration",
+                "org.apache.camel.component.consul.cloud.ConsulServiceRegistryConfiguration");
+    }
+
+    /* Make the build fail as long as there are banned classes registered for reflection */
+    @BuildStep
+    void bannedReflectiveClasses(
+            CombinedIndexBuildItem combinedIndex,
+            List<ReflectiveClassBuildItem> reflectiveClasses,
+            List<UnbannedReflectiveBuildItem> unbannedReflectives,
+            BuildProducer<GeneratedResourceBuildItem> dummy // to force the execution of this method
+    ) {
+        final DotName uriParamsDotName = DotName.createSimple("org.apache.camel.spi.UriParams");
+
+        final Set<String> bannedClassNames = combinedIndex.getIndex()
+                .getAnnotations(uriParamsDotName)
+                .stream()
+                .filter(ai -> ai.target().kind() == Kind.CLASS)
+                .map(ai -> ai.target().asClass().name().toString())
+                .collect(Collectors.toSet());
+
+        final Set<String> unbannedClassNames = unbannedReflectives.stream()
+                .map(UnbannedReflectiveBuildItem::getClassNames)
+                .flatMap(Collection::stream)
+                .collect(Collectors.toSet());
+
+        Set<String> violations = reflectiveClasses.stream()
+                .map(ReflectiveClassBuildItem::getClassNames)
+                .flatMap(Collection::stream)
+                .filter(cl -> !unbannedClassNames.contains(cl))
+                .filter(bannedClassNames::contains)
+                .collect(Collectors.toSet());
+
+        if (!violations.isEmpty()) {
+            throw new IllegalStateException(
 
 Review comment:
   Yes, this is only with our tests. It won't happen at runtime (unless the user explictly depends on the new policy extension)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
12