Copilot commented on code in PR #16248:
URL: https://github.com/apache/grails-core/pull/16248#discussion_r3882669682
##########
grails-forge/grails-forge-web-netty/build.gradle:
##########
@@ -124,20 +123,23 @@ tasks.named('dockerBuild') {
images = [findProperty('dockerImageName') ?: 'grailsforge']
}
-TaskProvider<ShadowJar> shadowJarTask = tasks.named('shadowJar', ShadowJar)
-shadowJarTask.configure {
- reproducibleFileOrder = true
- preserveFileTimestamps = false
+['com.gradleup.shadow', 'com.github.johnrengelman.shadow'].each { pluginId ->
+ pluginManager.withPlugin(pluginId) {
+ tasks.named('shadowJar', ShadowJar).configure {
+ reproducibleFileOrder = true
+ preserveFileTimestamps = false
+ }
+ }
}
TaskProvider<Zip> awsElasticBeanstalk = tasks.register('awsElasticBeanstalk',
Zip) {
- dependsOn(shadowJarTask)
+ dependsOn('shadowJar')
archiveFileName.set('grails-forge-web-netty-aws.zip')
destinationDirectory.set(layout.buildDirectory.dir('distributions'))
reproducibleFileOrder = true
preserveFileTimestamps = false
- from(shadowJarTask.flatMap { it.archiveFile }) {
+ from({ tasks.named('shadowJar').get().archiveFile.get() }) {
rename { 'app.jar' }
}
Review Comment:
This uses `tasks.named('shadowJar').get().archiveFile.get()` which eagerly
realizes the task and resolves the provider, reducing Gradle’s lazy
configuration benefits and making configuration-cache friendliness harder.
Prefer wiring the archive as a provider (e.g., via a typed
`TaskProvider<ShadowJar>` and `flatMap { it.archiveFile }`) and passing the
provider directly to `from(...)`.
##########
grails-forge/grails-forge-web-netty/build.gradle:
##########
@@ -124,20 +123,23 @@ tasks.named('dockerBuild') {
images = [findProperty('dockerImageName') ?: 'grailsforge']
}
-TaskProvider<ShadowJar> shadowJarTask = tasks.named('shadowJar', ShadowJar)
-shadowJarTask.configure {
- reproducibleFileOrder = true
- preserveFileTimestamps = false
+['com.gradleup.shadow', 'com.github.johnrengelman.shadow'].each { pluginId ->
+ pluginManager.withPlugin(pluginId) {
+ tasks.named('shadowJar', ShadowJar).configure {
+ reproducibleFileOrder = true
+ preserveFileTimestamps = false
+ }
+ }
}
TaskProvider<Zip> awsElasticBeanstalk = tasks.register('awsElasticBeanstalk',
Zip) {
- dependsOn(shadowJarTask)
+ dependsOn('shadowJar')
Review Comment:
`awsElasticBeanstalk` depends on `shadowJar` unconditionally, but this file
no longer applies a Shadow plugin explicitly. If neither Shadow plugin is
applied by other plugins, Gradle will fail with “Task 'shadowJar' not found.”
Consider either (a) applying the Shadow plugin explicitly in this build, or (b)
registering/configuring `awsElasticBeanstalk` inside
`pluginManager.withPlugin(...)` so the task only exists when `shadowJar` exists.
##########
.github/workflows/forge-deploy-aws.yml:
##########
@@ -123,85 +86,42 @@ jobs:
run: |
set -euo pipefail
case "${SLOT}" in
- latest)
- hostname='latest.grails.org'
- ;;
- snapshot)
- hostname='snapshot.grails.org'
- ;;
- next)
- hostname='next.grails.org'
- ;;
- prev)
- hostname='prev.grails.org'
- ;;
- prev-snapshot)
- hostname='prev-snapshot.grails.org'
- ;;
+ latest) hostname='latest.grails.org' ;;
+ snapshot) hostname='snapshot.grails.org' ;;
+ next) hostname='next.grails.org' ;;
+ prev) hostname='prev.grails.org' ;;
+ prev-snapshot) hostname='prev-snapshot.grails.org' ;;
*)
echo "Unsupported deployment slot" >&2
exit 1
;;
esac
-
- sanitized_release="$(printf '%s' "${RELEASE}" | tr '[:upper:]'
'[:lower:]' | tr -cs 'a-z0-9._-' '-')"
- sanitized_release="${sanitized_release#-}"
- sanitized_release="${sanitized_release%-}"
- label_prefix="${SLOT}"
- if [[ -n "${sanitized_release}" ]]; then
- label_prefix="${label_prefix}-${sanitized_release}"
- fi
source_sha="$(git rev-parse HEAD)"
-
label_suffix="-${source_sha:0:12}-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}"
- label_prefix="${label_prefix:0:$((100 - ${#label_suffix}))}"
- version_label="${label_prefix%-}${label_suffix}"
-
+
version_label="${SLOT}-${source_sha:0:12}-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}"
printf 'hostname=%s\n' "${hostname}" >> "${GITHUB_OUTPUT}"
printf 'version-label=%s\n' "${version_label}" >> "${GITHUB_OUTPUT}"
printf 's3-key=artifacts/%s/%s.zip\n' "${SLOT}" "${version_label}"
>> "${GITHUB_OUTPUT}"
- - name: "Test and package Forge web application"
+ - name: "Package Forge for Elastic Beanstalk"
working-directory: grails-forge
shell: bash
- run: >
- ./gradlew
- grails-forge-api:test
- grails-forge-web-netty:test
- grails-forge-web-netty:awsElasticBeanstalk
- env:
- TEST_BUILD_REPRODUCIBLE: "true"
-
- - name: "Verify Elastic Beanstalk package"
- shell: bash
- run: |
- set -euo pipefail
-
artifact='grails-forge/grails-forge-web-netty/build/distributions/grails-forge-web-netty-aws.zip'
- test -s "${artifact}"
-
- - name: "Verify AWS OIDC role configuration"
- shell: bash
- run: |
- set -euo pipefail
- if [[ -z "${DEPLOY_ROLE_ARN}" ]]; then
- echo "AWS_FORGE_DEPLOY_ROLE_ARN must be configured as a repository
variable." >&2
- exit 1
- fi
- if [[ ! "${DEPLOY_ROLE_ARN}" =~ ^arn:aws:iam::[0-9]{12}:role/.+$ ]];
then
- echo "AWS_FORGE_DEPLOY_ROLE_ARN must be an IAM role ARN." >&2
- exit 1
- fi
+ run: ./gradlew grails-forge-web-netty:awsElasticBeanstalk
- name: "Configure AWS credentials through OIDC"
uses:
aws-actions/configure-aws-credentials@e7f100cf4c008499ea8adda475de1042d6975c7b
# v6.2.0
with:
role-to-assume: ${{ env.DEPLOY_ROLE_ARN }}
- aws-region: ${{ inputs.aws_region }}
+ aws-region: us-east-1
- name: "Discover shared deployment resources"
id: shared
shell: bash
run: |
set -euo pipefail
+ if [[ -z "${DEPLOY_ROLE_ARN}" ]]; then
+ echo "AWS_FORGE_DEPLOY_ROLE_ARN must be configured as a repository
variable." >&2
+ exit 1
+ fi
Review Comment:
`DEPLOY_ROLE_ARN` is validated only *after* `configure-aws-credentials`
runs. If the variable is missing/empty, the workflow will fail inside the
action with a less clear error and won’t hit your explicit message. Move this
validation step before “Configure AWS credentials through OIDC” so failures are
immediate and actionable.
##########
.github/workflows/forge-deploy-aws.yml:
##########
@@ -13,13 +13,11 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-# GitHub registers workflow_dispatch from the default branch. Dispatch from
that
-# branch and set source_ref to the maintenance line or tag to build. The job
-# reads javaVersion from gradle.properties. OIDC trusts refs/heads/*.x and
-# refs/tags/v* in apache/grails-core, so new version branches need no template
edit.
+# Dispatch from the maintenance branch that should be built (Use workflow
from).
+# Choose the slot. Region, stack name, and JDK come from that branch.
Review Comment:
The comment says “Region, stack name … come from that branch,” but the
workflow now hard-codes `AWS_REGION`/`AWS_DEFAULT_REGION` to `us-east-1` and
`SHARED_STACK` to `grails-forge-shared`. Update this header comment to match
the current behavior (branch controls source/JDK; region/stack are fixed).
##########
.github/workflows/forge-deploy-aws.yml:
##########
@@ -276,14 +183,8 @@ jobs:
local exit_code=$?
trap - ERR
if [[ "${update_started}" == true && "${rollback_available}" ==
true ]]; then
- echo "Deployment failed. Rolling back ${ENVIRONMENT_NAME} to its
previous application version."
- if aws elasticbeanstalk update-environment --environment-name
"${ENVIRONMENT_NAME}" --version-label "${previous_version}" >/dev/null &&
wait_for_ready_green 'Rollback' "${previous_version}"; then
- echo "Rollback completed."
- else
- echo "Rollback did not reach Status=Ready and Health=Green.
Manual intervention is required." >&2
- fi
- elif [[ "${update_started}" == true ]]; then
- echo "Deployment failed, but no prior application version is
available for rollback." >&2
+ echo "Deployment failed. Rolling back ${ENVIRONMENT_NAME}."
+ aws elasticbeanstalk update-environment --environment-name
"${ENVIRONMENT_NAME}" --version-label "${previous_version}" >/dev/null || true
Review Comment:
The rollback path now suppresses rollback failures (`|| true`) and no longer
waits for the environment to return to `Ready/Green`. That can mask rollback
problems and leave the environment mid-transition without visibility. Consider
keeping a best-effort rollback, but (1) log failures explicitly, and (2) reuse
`wait_for_ready_green` for rollback (or at least wait for `Status=Ready`) so
the workflow provides a reliable rollback outcome.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
