Copilot commented on code in PR #16240: URL: https://github.com/apache/grails-core/pull/16240#discussion_r3881890015
########## .github/workflows/forge-deploy-aws.yml: ########## @@ -0,0 +1,330 @@ +# 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 +# +# https://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. +# +# 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. + +name: "Forge - AWS Elastic Beanstalk Deploy" +run-name: "Forge AWS ${{ inputs.slot }} ${{ inputs.source_ref || github.ref_name }}${{ inputs.release && format(' - {0}', inputs.release) || '' }}" + +on: + workflow_dispatch: + inputs: + slot: + description: 'Deployment slot' + required: true + type: choice + options: + - latest + - snapshot + - next + - prev + - prev-snapshot + source_ref: + description: 'Git branch or tag to build. Leave empty to build the workflow ref.' + required: false + type: string + release: + description: 'Optional release identifier for the deployment label' + required: false + type: string + shared_stack: + description: 'CloudFormation stack exporting Forge deployment resources' + default: grails-forge-shared + required: false + type: string + aws_region: + description: 'AWS region containing the shared stack and Elastic Beanstalk application' + default: us-east-1 + required: false + type: string + +permissions: + contents: read + id-token: write + +concurrency: + group: forge-aws-deploy-${{ inputs.slot }} + cancel-in-progress: false + +jobs: + deploy: + name: "Deploy ${{ inputs.slot }} to AWS Elastic Beanstalk" + runs-on: ubuntu-24.04 + timeout-minutes: 75 + env: + AWS_REGION: ${{ inputs.aws_region }} + AWS_DEFAULT_REGION: ${{ inputs.aws_region }} + AWS_PAGER: "" + DEPLOY_ROLE_ARN: ${{ vars.AWS_FORGE_DEPLOY_ROLE_ARN }} + RELEASE: ${{ inputs.release }} + SHARED_STACK: ${{ inputs.shared_stack }} + SLOT: ${{ inputs.slot }} + SOURCE_REF: ${{ inputs.source_ref || github.ref_name }} + steps: + - name: "Validate source ref" + shell: bash + run: | + set -euo pipefail + if [[ "${SOURCE_REF}" =~ ^[0-9]+\.[0-9]+\.x$ ]]; then + exit 0 + fi + if [[ "${SOURCE_REF}" =~ ^v[0-9][A-Za-z0-9._-]*$ ]]; then + exit 0 + fi + echo "source_ref must be a maintenance branch such as 7.1.x or a tag such as v7.1.6." >&2 + exit 1 Review Comment: The `source_ref` input is documented as optional (\"Leave empty to build the workflow ref\"), but the validation step rejects any ref that isn't `*.x` or `v*`. When dispatching from the default branch (commonly `main`), leaving `source_ref` empty will fail immediately. Make this consistent by either (a) making `source_ref` required and updating the description/message accordingly, or (b) allowing the workflow ref when `inputs.source_ref` is empty (and ensuring the IAM trust policy also permits that ref if deployment is intended). ########## .github/workflows/forge-deploy-aws.yml: ########## @@ -0,0 +1,330 @@ +# 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 +# +# https://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. +# +# 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. + +name: "Forge - AWS Elastic Beanstalk Deploy" +run-name: "Forge AWS ${{ inputs.slot }} ${{ inputs.source_ref || github.ref_name }}${{ inputs.release && format(' - {0}', inputs.release) || '' }}" + +on: + workflow_dispatch: + inputs: + slot: + description: 'Deployment slot' + required: true + type: choice + options: + - latest + - snapshot + - next + - prev + - prev-snapshot + source_ref: + description: 'Git branch or tag to build. Leave empty to build the workflow ref.' + required: false + type: string Review Comment: The `source_ref` input is documented as optional (\"Leave empty to build the workflow ref\"), but the validation step rejects any ref that isn't `*.x` or `v*`. When dispatching from the default branch (commonly `main`), leaving `source_ref` empty will fail immediately. Make this consistent by either (a) making `source_ref` required and updating the description/message accordingly, or (b) allowing the workflow ref when `inputs.source_ref` is empty (and ensuring the IAM trust policy also permits that ref if deployment is intended). ########## grails-forge/infrastructure/environment.yaml: ########## @@ -0,0 +1,291 @@ +# SPDX-License-Identifier: Apache-2.0 +# +# 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. +AWSTemplateFormatVersion: '2010-09-09' +Description: One Grails Forge Elastic Beanstalk API environment behind the shared ALB. + +Parameters: + SharedStackName: + Type: String + Default: grails-forge-shared + Description: Name of the shared Grails Forge infrastructure stack. + Slot: + Type: String + AllowedValues: + - latest + - snapshot + - next + - prev + - prev-snapshot + Description: Deployment slot. One environment stack is required for each slot. + HostName: + Type: String + Description: Exact DNS hostname matched by the shared HTTPS listener rule. + CorsAllowedOrigin: + Type: String + Default: https://start.grails.org + Description: Browser origin permitted by the API CORS policy. + ListenerRulePriority: + Type: Number + MinValue: 1 + MaxValue: 50000 + Description: Unique shared HTTPS listener rule priority for this slot. + InstanceSubnets: + Type: List<AWS::EC2::Subnet::Id> + Description: Public instance subnets for this Elastic Beanstalk environment. + PlatformArn: + Type: String + Description: Required concrete Elastic Beanstalk Corretto 25 platform ARN for the selected architecture. Review Comment: PR description says this merge \"Keeps ... Java 21\", but the infrastructure templates/docs are explicitly calling for Corretto/Java 25 (e.g., this `PlatformArn` description, and the infra README/runbook text). This is likely to confuse operators and may cause accidental deployment onto the wrong runtime. Align the templates + documentation with the intended Java version (either update to Corretto 21 / Java 21 throughout, or update the PR description to reflect Java 25). ########## grails-forge/infrastructure/environment.yaml: ########## @@ -0,0 +1,291 @@ +# SPDX-License-Identifier: Apache-2.0 +# +# 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. +AWSTemplateFormatVersion: '2010-09-09' +Description: One Grails Forge Elastic Beanstalk API environment behind the shared ALB. + +Parameters: + SharedStackName: + Type: String + Default: grails-forge-shared + Description: Name of the shared Grails Forge infrastructure stack. + Slot: + Type: String + AllowedValues: + - latest + - snapshot + - next + - prev + - prev-snapshot + Description: Deployment slot. One environment stack is required for each slot. + HostName: + Type: String + Description: Exact DNS hostname matched by the shared HTTPS listener rule. + CorsAllowedOrigin: + Type: String + Default: https://start.grails.org + Description: Browser origin permitted by the API CORS policy. + ListenerRulePriority: + Type: Number + MinValue: 1 + MaxValue: 50000 + Description: Unique shared HTTPS listener rule priority for this slot. + InstanceSubnets: + Type: List<AWS::EC2::Subnet::Id> + Description: Public instance subnets for this Elastic Beanstalk environment. + PlatformArn: + Type: String + Description: Required concrete Elastic Beanstalk Corretto 25 platform ARN for the selected architecture. + InstanceType: + Type: String + Default: t4g.small + Description: EC2 instance type for the API environment. + Architecture: + Type: String + AllowedValues: + - arm64 + - x86_64 + Default: arm64 + Description: Instance architecture supported by PlatformArn and InstanceType. + MinSize: + Type: Number + Default: 1 + MinValue: 1 + Description: Minimum API instances. + MaxSize: + Type: Number + Default: 2 + MinValue: 1 + Description: Maximum API instances. + GitHubRedirectUrl: + Type: String + Default: https://start.grails.org/ + Description: Browser redirect target for the Forge UI. + +Resources: + Environment: + Type: AWS::ElasticBeanstalk::Environment + Properties: + ApplicationName: + Fn::ImportValue: + Fn::Sub: ${SharedStackName}:ApplicationName + Description: + Fn::Sub: Grails Forge ${Slot} API environment. + EnvironmentName: + Fn::Sub: + - ${ApplicationName}-${Slot} + - ApplicationName: + Fn::ImportValue: + Fn::Sub: ${SharedStackName}:ApplicationName + PlatformArn: + Ref: PlatformArn + OptionSettings: + - Namespace: aws:autoscaling:asg + OptionName: MinSize + Value: + Fn::Sub: ${MinSize} + - Namespace: aws:autoscaling:asg + OptionName: MaxSize + Value: + Fn::Sub: ${MaxSize} + - Namespace: aws:autoscaling:launchconfiguration + OptionName: DisableDefaultEC2SecurityGroup + Value: 'true' + - Namespace: aws:autoscaling:launchconfiguration + OptionName: DisableIMDSv1 + Value: 'true' + - Namespace: aws:autoscaling:launchconfiguration + OptionName: IamInstanceProfile + Value: + Fn::ImportValue: + Fn::Sub: ${SharedStackName}:InstanceProfileName + - Namespace: aws:autoscaling:launchconfiguration + OptionName: RootVolumeType + Value: gp3 + - Namespace: aws:autoscaling:launchconfiguration + OptionName: SecurityGroups + Value: + Fn::ImportValue: + Fn::Sub: ${SharedStackName}:InstanceSecurityGroupId + - Namespace: aws:autoscaling:trigger + OptionName: BreachDuration + Value: '5' + - Namespace: aws:autoscaling:trigger + OptionName: LowerThreshold + Value: '20' + - Namespace: aws:autoscaling:trigger + OptionName: MeasureName + Value: CPUUtilization + - Namespace: aws:autoscaling:trigger + OptionName: Period + Value: '1' + - Namespace: aws:autoscaling:trigger + OptionName: Statistic + Value: Average + - Namespace: aws:autoscaling:trigger + OptionName: Unit + Value: Percent + - Namespace: aws:autoscaling:trigger + OptionName: UpperThreshold + Value: '70' + - Namespace: aws:ec2:vpc + OptionName: AssociatePublicIpAddress + Value: 'true' Review Comment: Forcing public IP assignment increases operational/security exposure and ties deployments to public subnets, even though inbound traffic should only come via the ALB. If feasible, prefer private subnets for EB instances (disable public IPs and use NAT/VPC endpoints as needed). If public subnets are a hard requirement for this migration, consider documenting that rationale explicitly in the infra README/runbook. ########## grails-forge/infrastructure/shared.yaml: ########## @@ -0,0 +1,376 @@ +# SPDX-License-Identifier: Apache-2.0 +# +# 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. +AWSTemplateFormatVersion: '2010-09-09' +Description: Shared AWS resources for Grails Forge Elastic Beanstalk environments. + +Parameters: + VpcId: + Type: AWS::EC2::VPC::Id + Description: VPC containing the shared load balancer and Elastic Beanstalk instances. + PublicSubnets: + Type: List<AWS::EC2::Subnet::Id> + Description: At least two public subnets for the internet-facing application load balancer. + CertificateArn: + Type: String + Description: ACM certificate ARN for the Grails Forge HTTPS listener. + GitHubOidcProviderArn: + Type: String + Description: ARN of the GitHub Actions IAM OpenID Connect provider. + GitHubRepository: + Type: String + Default: apache/grails-core + Description: GitHub repository trusted to assume the deployment role. + ApplicationName: + Type: String + Default: grails-forge + Description: Elastic Beanstalk application name. + ArtifactRetentionDays: + Type: Number + Default: 90 + MinValue: 1 + Description: Days to retain application versions and their deployment artifacts. + +Resources: + ArtifactBucket: + Type: AWS::S3::Bucket + DeletionPolicy: Retain + UpdateReplacePolicy: Retain + Properties: + BucketEncryption: + ServerSideEncryptionConfiguration: + - ServerSideEncryptionByDefault: + SSEAlgorithm: AES256 + LifecycleConfiguration: + Rules: + - Id: ExpireNoncurrentArtifacts + NoncurrentVersionExpiration: + NoncurrentDays: + Ref: ArtifactRetentionDays + Status: Enabled + PublicAccessBlockConfiguration: + BlockPublicAcls: true + BlockPublicPolicy: true + IgnorePublicAcls: true + RestrictPublicBuckets: true + VersioningConfiguration: + Status: Enabled + + AlbSecurityGroup: + Type: AWS::EC2::SecurityGroup + Properties: + GroupDescription: Public HTTPS and HTTP redirect access for Grails Forge. + SecurityGroupIngress: + - CidrIp: 0.0.0.0/0 + Description: HTTPS access from the internet. + FromPort: 443 + IpProtocol: tcp + ToPort: 443 + - CidrIp: 0.0.0.0/0 + Description: HTTP access redirected to HTTPS. + FromPort: 80 + IpProtocol: tcp + ToPort: 80 + VpcId: + Ref: VpcId + + SharedLoadBalancer: + Type: AWS::ElasticLoadBalancingV2::LoadBalancer + Properties: + IpAddressType: ipv4 + Scheme: internet-facing + SecurityGroups: + - Ref: AlbSecurityGroup + Subnets: + Ref: PublicSubnets + Type: application + + HttpsListener: + Type: AWS::ElasticLoadBalancingV2::Listener + Properties: + Certificates: + - CertificateArn: + Ref: CertificateArn + DefaultActions: + - FixedResponseConfig: + ContentType: text/plain + MessageBody: Not Found + StatusCode: '404' + Type: fixed-response + LoadBalancerArn: + Ref: SharedLoadBalancer + Port: 443 + Protocol: HTTPS + SslPolicy: ELBSecurityPolicy-TLS13-1-2-2021-06 + + HttpListener: + Type: AWS::ElasticLoadBalancingV2::Listener + Properties: + DefaultActions: + - RedirectConfig: + Port: '443' + Protocol: HTTPS + StatusCode: HTTP_301 + Type: redirect + LoadBalancerArn: + Ref: SharedLoadBalancer + Port: 80 + Protocol: HTTP + + ElasticBeanstalkApplication: + Type: AWS::ElasticBeanstalk::Application + Properties: + ApplicationName: + Ref: ApplicationName + Description: Grails Forge Micronaut API. + ResourceLifecycleConfig: + ServiceRole: + Fn::GetAtt: + - ElasticBeanstalkServiceRole + - Arn + VersionLifecycleConfig: + MaxAgeRule: + DeleteSourceFromS3: true + Enabled: true + MaxAgeInDays: + Ref: ArtifactRetentionDays + + ElasticBeanstalkServiceRole: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Statement: + - Action: sts:AssumeRole + Effect: Allow + Principal: + Service: + - elasticbeanstalk.amazonaws.com + - managedupdates.elasticbeanstalk.amazonaws.com + Version: '2012-10-17' + ManagedPolicyArns: + - Fn::Sub: arn:${AWS::Partition}:iam::aws:policy/service-role/AWSElasticBeanstalkEnhancedHealth + - Fn::Sub: arn:${AWS::Partition}:iam::aws:policy/AWSElasticBeanstalkManagedUpdatesCustomerRolePolicy + Policies: + - PolicyDocument: + Statement: + - Action: elasticbeanstalk:DeleteApplicationVersion + Effect: Allow + Resource: + Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:applicationversion/${ApplicationName}/* + - Action: s3:DeleteObject + Effect: Allow + Resource: + Fn::Sub: ${ArtifactBucket.Arn}/* + Version: '2012-10-17' + PolicyName: DeleteExpiredGrailsForgeSourceBundles + + ElasticBeanstalkInstanceRole: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Statement: + - Action: sts:AssumeRole + Effect: Allow + Principal: + Service: ec2.amazonaws.com + Version: '2012-10-17' + ManagedPolicyArns: + - Fn::Sub: arn:${AWS::Partition}:iam::aws:policy/AWSElasticBeanstalkWebTier + - Fn::Sub: arn:${AWS::Partition}:iam::aws:policy/AmazonSSMManagedInstanceCore + Policies: + - PolicyDocument: + Statement: + - Action: + - s3:GetObject + - s3:GetObjectVersion + Effect: Allow + Resource: + Fn::Sub: ${ArtifactBucket.Arn}/* + Version: '2012-10-17' + PolicyName: ReadGrailsForgeSourceBundles + + ElasticBeanstalkInstanceProfile: + Type: AWS::IAM::InstanceProfile + Properties: + Roles: + - Ref: ElasticBeanstalkInstanceRole + + InstanceSecurityGroup: + Type: AWS::EC2::SecurityGroup + Properties: + GroupDescription: Grails Forge Elastic Beanstalk instances reachable only through the ALB. + SecurityGroupIngress: + - Description: HTTP traffic from the shared application load balancer. + FromPort: 80 + IpProtocol: tcp + SourceSecurityGroupId: + Ref: AlbSecurityGroup + ToPort: 80 + VpcId: + Ref: VpcId + + GitHubDeployRole: + Type: AWS::IAM::Role + Properties: + AssumeRolePolicyDocument: + Statement: + - Action: sts:AssumeRoleWithWebIdentity + Condition: + StringEquals: + token.actions.githubusercontent.com:aud: sts.amazonaws.com + StringLike: + token.actions.githubusercontent.com:sub: + - Fn::Sub: repo:${GitHubRepository}:ref:refs/heads/*.x + - Fn::Sub: repo:${GitHubRepository}:ref:refs/tags/v* + Effect: Allow + Principal: + Federated: + Ref: GitHubOidcProviderArn + Version: '2012-10-17' + Policies: + - PolicyDocument: + Statement: + - Action: + - s3:GetObject + - s3:GetObjectVersion + - s3:PutObject + Effect: Allow + Resource: + Fn::Sub: ${ArtifactBucket.Arn}/* + - Action: + - elasticbeanstalk:CreateApplicationVersion + Effect: Allow + Resource: + - Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:application/${ApplicationName} + - Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:applicationversion/${ApplicationName}/* + - Action: elasticbeanstalk:DescribeApplicationVersions + Effect: Allow + Resource: + Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:applicationversion/${ApplicationName}/* + - Action: elasticbeanstalk:DescribeApplications + Effect: Allow + Resource: + Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:application/${ApplicationName} + - Action: + - elasticbeanstalk:DescribeEnvironments + - elasticbeanstalk:UpdateEnvironment + Effect: Allow + Resource: + - Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:environment/${ApplicationName}/${ApplicationName}-latest + - Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:environment/${ApplicationName}/${ApplicationName}-snapshot + - Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:environment/${ApplicationName}/${ApplicationName}-next + - Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:environment/${ApplicationName}/${ApplicationName}-prev + - Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:environment/${ApplicationName}/${ApplicationName}-prev-snapshot + - Fn::Sub: arn:${AWS::Partition}:elasticbeanstalk:${AWS::Region}:${AWS::AccountId}:applicationversion/${ApplicationName}/* + - Action: cloudformation:DescribeStacks + Effect: Allow + Resource: '*' Review Comment: `cloudformation:DescribeStacks` is granted on `Resource: '*'`, which broadens read access beyond what's needed for this workflow. Prefer scoping to the specific shared stack ARN (and any other stack(s) that must be readable) if the action supports resource-level permissions in your account/partition; otherwise add a brief comment explaining why wildcard is required. ########## grails-forge/grails-forge-api/src/test/groovy/org/grails/forge/api/ApplicationControllerSpec.groovy: ########## @@ -50,6 +55,24 @@ class ApplicationControllerSpec extends Specification { response.versions["grails.version"] } + void "test redirect URL configuration"() { + expect: + configuration.redirectUri.get().toString() == 'https://example.com/forge/' + } + + void "test absent redirect URL configuration ignores null"() { Review Comment: This test name is misleading: it doesn't test an 'absent redirect URL configuration' (the configuration appears to have a non-null default), it tests that assigning `null` does not overwrite the existing value. Rename the test to reflect the actual behavior under test (e.g., 'null redirectUrl does not overwrite existing/default redirectUrl'). -- 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]
