gemmellr commented on code in PR #5245:
URL: https://github.com/apache/activemq-artemis/pull/5245#discussion_r1775277300


##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Upgrade.java:
##########
@@ -148,7 +152,7 @@ public Object run(ActionContext context) throws Exception {
                  "<env name=\"ARTEMIS_INSTANCE\"", "<env 
name=\"ARTEMIS_INSTANCE_ETC\"",
                  "<env name=\"ARTEMIS_INSTANCE_URI\"", "<env 
name=\"ARTEMIS_INSTANCE_ETC_URI\"",
                  "<env name=\"ARTEMIS_DATA_DIR\"", "<logpath>", 
"<startargument>-Xmx", "<stopargument>-Xmx",
-                 "<name>", "<id>", "<startargument>-Dhawtio.role=");
+                 "<name>", "<id>", "<startargument>-Dhawtio.roles=");

Review Comment:
   This isnt going to properly upgrade hawtio.role into hawtio.roles.
   
   The changes I made in the previous PR against 2.36.0 already handled both 
upgrading the hawtio.role value and keeping hawtio.roles.
   
   



##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Upgrade.java:
##########
@@ -194,10 +198,10 @@ public Object run(ActionContext context) throws Exception 
{
          File artemisProfile = new File(etcFolder, Create.ETC_ARTEMIS_PROFILE);
          File artemisProfileTmp = new File(tmp, Create.ETC_ARTEMIS_PROFILE);
          File artemisProfileBkp = new File(etcBkp, Create.ETC_ARTEMIS_PROFILE);
-
          write("etc/" + Create.ETC_ARTEMIS_PROFILE, artemisProfileTmp, 
filters, false, false);
          upgradeJDK(context, JDK_PREFIX_LINUX, "\"", KEEPING_JVM_ARGUMENTS, 
artemisProfileTmp, artemisProfile, artemisProfileBkp,
                "ARTEMIS_INSTANCE=", "ARTEMIS_DATA_DIR=", "ARTEMIS_ETC_DIR=", 
"ARTEMIS_OOME_DUMP=", "ARTEMIS_INSTANCE_URI=", "ARTEMIS_INSTANCE_ETC_URI=", 
"HAWTIO_ROLE=");
+         replaceLines(context, artemisProfileTmp, artemisProfile, 
artemisProfileBkp, "^HAWTIO_ROLE(.*)$,HAWTIO_ROLES$1");

Review Comment:
   This seems wrong, as the upgrade will have just written a new profile with 
new HAWTIO_ROLES value, most probably with the wrong value, on the line 
above....so this should be an effective no-op.
   
   This seems like it will also fail overall to upgrade current config with 
HAWTIO_ROLES already in it, since the line above doesnt keep HAWTIO_ROLES 
values like it should, only old HAWTIO_ROLE values.
   
   It also seems like it could overwrite the original backup with the 
already-part-upgraded 'new original' file, losing the 'true original', which if 
so is definitely not what we want.
   
   The changes I made in the previous PR against 2.36.0 already handled both 
upgrading the HAWTIO_ROLE value and keeping HAWTIO_ROLES, and from having a 
single step should have a single backup.



##########
tests/e2e-tests/src/main/resources/containerService/artemis:
##########
@@ -44,7 +44,7 @@ if [ -z "$ARTEMIS_INSTANCE" ] ; then
   ARTEMIS_INSTANCE=`cd "$ARTEMIS_INSTANCE/.." && pwd`
 fi
 
-HAWTIO_ROLE="NO_HAWTIO_ROLE"
+HAWTIO_ROLES="NO_HAWTIO_ROLE"

Review Comment:
   Same as last PR originally, you should not be changing this pre-upgrade 
config in this way. The whole point of this config is to check the upgrade 
command actually updates it properly (which per earlier comments, it seems it 
doesnt, which is probably why you needed to make this hack again to pass the 
test pointing it out)



##########
artemis-distribution/src/main/resources/licenses/bin/LICENSE:
##########
@@ -309,163 +309,19 @@ This subcomponent is bundled by default but is optional.
 It bundles the below declared packages not covered by ASL V2.
 
 ==============================================================================
-For jquery:
+For JSON-java
 ==============================================================================
-This product bundles jquery, which is available under a "MIT" license.
-For details, see licenses/LICENSE-jquery.txt
+This product bundles the JSON Java API, which is available under a
+public domain license. for details see licenses/LICENSE-JSON-java.txt
 
 ==============================================================================
-For datatables:
+For checker-qual
 ==============================================================================
-This product bundles datatables, which is available under a "MIT" license.
-For details, see licenses/LICENSE-datatables.txt
+This product bundles the checker qual API which is available under
+the MIT license, for details see licenses/LICENSE-checker-qual.txt
 
 ==============================================================================
-For bootstrap:
+For all the NPM dependencies
 ==============================================================================
-This product bundles bootstrap, which is available under a "MIT" license.
-For details, see licenses/LICENSE-bootstrap.txt
-
-==============================================================================
-For patternfly:
-==============================================================================
-This product bundles patternfly, which is available under a "MIT" license.
-For details, see licenses/LICENSE-patternfly.txt
-
-==============================================================================
-For angular:
-==============================================================================
-This product bundles angular, which is available under a "MIT" license.
-For details, see licenses/LICENSE-angular.txt
-
-==============================================================================
-For angular-animate:
-==============================================================================
-This product bundles angular-animate, which is available under a "MIT" license.
-For details, see licenses/LICENSE-angular-animate.txt
-
-==============================================================================
-For angular-sanitize:
-==============================================================================
-This product bundles angular-sanitize, which is available under a "MIT" 
license.
-For details, see licenses/LICENSE-angular-sanitize.txt
-
-==============================================================================
-For angular-route:
-==============================================================================
-This product bundles angular-route, which is available under a "MIT" license.
-For details, see licenses/LICENSE-angular-route.txt
-
-==============================================================================
-For angular-ui-bootstrap:
-==============================================================================
-This product bundles angular-ui-bootstrap, which is available under a "MIT" 
license.
-For details, see licenses/LICENSE-angular-ui-bootstrap.txt
-
-==============================================================================
-For angular-resizable:
-==============================================================================
-This product bundles angular-resizable, which is available under a "MIT" 
license.
-For details, see licenses/LICENSE-angular-resizable.txt
-
-==============================================================================
-For angular-drag-and-drop-lists:
-==============================================================================
-This product bundles angular-drag-and-drop-lists, which is available under a 
"MIT" license.
-For details, see licenses/LICENSE-angular-drag-and-drop-lists.txt
-
-==============================================================================
-For angular-datatables:
-==============================================================================
-This product bundles angular-datatables, which is available under a "MIT" 
license.
-For details, see licenses/LICENSE-angular-datatables.txt
-
-==============================================================================
-For angular-cookies:
-==============================================================================
-This product bundles angular-cookies, which is available under a "MIT" license.
-For details, see licenses/LICENSE-angular-cookies.txt
-
-==============================================================================
-For angular-patternfly:
-==============================================================================
-This product bundles angular-patternfly, which is available under a "MIT" 
license.
-For details, see licenses/LICENSE-angular-patternfly.txt
-
-==============================================================================
-For angular-idle:
-==============================================================================
-This product bundles angular-idle, which is available under a "MIT" license.
-For details, see licenses/LICENSE-angular-idle.txt
-
-==============================================================================
-For c3:
-==============================================================================
-This product bundles c3, which is available under a "MIT" license.
-For details, see licenses/LICENSE-c3.txt
-
-==============================================================================
-For d3:
-==============================================================================
-This product bundles d3, which is available under a "BSD 3-clause" license.
-For details, see licenses/LICENSE-d3.txt
-
-==============================================================================
-For lodash:
-==============================================================================
-This product bundles lodash, which is available under a "MIT" license.
-For details, see licenses/LICENSE-lodash.txt
-
-==============================================================================
-For urijs:
-==============================================================================
-This product bundles urijs, which is available under a "MIT" license.
-For details, see licenses/LICENSE-urijs.txt
-
-==============================================================================
-For js-logger:
-==============================================================================
-This product bundles js-logger, which is available under a "MIT" license.
-For details, see licenses/LICENSE-js-logger.txt
-
-==============================================================================
-For clipboard:
-==============================================================================
-This product bundles clipboard, which is available under a "MIT" license.
-For details, see licenses/LICENSE-clipboard.txt
-
-==============================================================================
-For marked:
-==============================================================================
-This product bundles marked, which is available under a "MIT" license.
-For details, see licenses/LICENSE-marked.txt
-
-==============================================================================
-For js-beautify:
-==============================================================================
-This product bundles js-beautify, which is available under a "MIT" license.
-For details, see licenses/LICENSE-js-beautify.txt
-
-==============================================================================
-For codemirror:
-==============================================================================
-This product bundles codemirror, which is available under a "MIT" license.
-For details, see licenses/LICENSE-codemirror.txt
-
-==============================================================================
-For graphlib:
-==============================================================================
-This product bundles graphlib, which is available under a "MIT" license.
-For details, see licenses/LICENSE-graphlib.txt
-
-==============================================================================
-For dagre:
-==============================================================================
-This product bundles dagre, which is available under a "MIT" license.
-For details, see licenses/LICENSE-dagre.txt
-
-==============================================================================
-For ng-infinite-scroll:
-==============================================================================
-This product bundles ng-infinite-scroll, which is available under a "MIT" 
license.
-For details, see licenses/LICENSE-ng-infinite-scroll.txt
+The licenses for dependencies sourced from NPM can be found in
+licenses/NPMLicenses.txt

Review Comment:
   Does this file exist? Dont see it being added, or actions to otherwise 
source it.



##########
artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/hawtio-oidc.properties:
##########
@@ -0,0 +1,76 @@
+#
+# 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.
+#
+
+# OpenID Connect configuration requred at client side
+
+# URL of OpenID Connect Provider - the URL after which 
".well-known/openid-configuration" can be appended for
+# discovery purposes
+# if this property is unavailable, OIDC is not enabled
+# provider = 
https://login.microsoftonline.com/00000000-1111-2222-3333-444444444444/v2.0
+# OpenID client identifier
+client_id = 00000000-1111-2222-3333-444444444444
+# response mode according to 
https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html
+response_mode = fragment
+# scope to request when performing OpenID authentication. MUST include 
"openid" and required permissions
+scope = openid email profile
+# redirect URI after OpenID authentication - must also be configured at 
provider side
+redirect_uri = http://localhost:8080/hawtio
+# challenge method according to https://datatracker.ietf.org/doc/html/rfc7636
+code_challenge_method = S256
+# prompt hint according to 
https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+prompt = login
+
+# additional configuration for the server side
+
+# if true, .well-known/openid-configuration will be fetched at server side. 
This is required
+# for proper JWT access token validation
+oidc.cacheConfig = false
+
+# time in minutes to cache public keys from jwks_uri
+jwks.cacheTime = 60
+
+# a path for an array of roles found in JWT payload. Property placeholders can 
be used for parameterized parts
+# of the path (like for Keycloak) - but only for properties from this 
particular file
+# example for properly configured Entra ID token
+oidc.rolesPath = roles
+# example for Keycloak with use-resource-role-mappings=true
+#oidc.rolesPath = resource_access.${client_id}.roles
+# example for Keycloak with use-resource-role-mappings=false
+#oidc.rolesPath = realm_access.roles
+
+# properties for role mapping. Each property with "roleMapping." prefix is 
used to map an original role
+# from JWT token (found at ${oidc.rolesPath}) to a role used by the application
+roleMapping.Hawtio.Admin = admin
+roleMapping.Hawtio.Manager = manager
+roleMapping.Hawtio.Viewer = viewer
+
+# timeout for connection establishment (milliseconds)
+http.connectionTimeout = 5000
+# timeout for reading from established connection (milliseconds)
+http.readTimeout = 10000
+# HTTP proxy to use when connecting to OpenID Connect provider
+#http.proxyURL = http://127.0.0.1:3128
+
+# TLS configuration (system properties can be used, e.g., 
"${catalina.home}/conf/hawtio.jks")
+
+ssl.protocol = TLSv1.3
+ssl.truststore = src/test/resources/hawtio.jks
+ssl.truststorePassword = hawtio
+ssl.keystore = src/test/resources/hawtio.jks
+ssl.keystorePassword = hawtio
+ssl.keyAlias = openid connect test provider
+ssl.keyPassword = hawtio

Review Comment:
   Some of these seem like questionable out-the-box property values to be 
defining



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact


Reply via email to