Skip to content

S3 deploy for Lambda services flag - #16

Open
Tanmay-Jupiter wants to merge 8 commits into
mainfrom
s3-deploy
Open

S3 deploy for Lambda services flag#16
Tanmay-Jupiter wants to merge 8 commits into
mainfrom
s3-deploy

Conversation

@Tanmay-Jupiter

@Tanmay-Jupiter Tanmay-Jupiter commented May 21, 2026

Copy link
Copy Markdown

PR Description

Added optional S3 deployment support for Lambda services in the deployment workflow.

Changes Made

  • Introduced conditional S3 deployment execution controlled via the s3_deploy flag.
  • Deployment step runs only when s3_deploy: true is passed by the caller.
  • Added support for building the deployable Lambda JAR using ./devops/build.sh.
  • Uploaded the generated deployment artifact to S3 as part of the workflow.

Benefits

  • Enables reusable deployment workflow for Lambda-based services.
  • Avoids unnecessary S3 upload steps for non-Lambda services.
  • Provides flexible and configurable deployment behavior through workflow inputs.

@jm-bot-ci

Copy link
Copy Markdown

@Tanmay-Jupiter: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jm-bot-ci

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Tanmay-Jupiter

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Comment thread .github/workflows/jvm-post-merge.yml Outdated
aws-secret-access-key: ${{ secrets.AWS_ACCESS_KEY_SECRET }}
aws-region: ap-south-1

- name: Set commit short sha

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use the logic from push action.

Comment thread .github/workflows/jvm-post-merge.yml Outdated
runs-on: ubuntu-latest

env:
GITHUB_TOKEN: ${{ secrets.CI_GITHUB_TOKEN }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

move to global env.


- name: Generate Service Metadata
id: metadata
if: inputs.s3_deploy == false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove

Comment thread .github/workflows/jvm-pre-merge.yml Outdated
# Builds the deployable JAR via ./devops/build.sh and uploads to S3.
# ─────────────────────────────────────────────────────────────────

s3-deploy:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

extract this job in an action and use it in both jvm-pre-merge and jvm-post-merge.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ack

@jm-bot-ci jm-bot-ci added size/L and removed size/M labels May 22, 2026
@rkp2024

rkp2024 commented May 22, 2026

Copy link
Copy Markdown

:robot_face: Jarvis cross-repo impact review

This PR adds a new composite action (.github/actions/s3-deploy/action.yml) for building shadow JARs and uploading them to S3, targeting Lambda services. It also extends both jvm-post-merge.yml and jvm-pre-merge.yml with three new optional inputs (s3_deploy, s3_bucket, app_modules) that gate an alternative s3-deploy job path — skipping the existing generate-metadata step and build-push (ECR) job when s3_deploy: true. All new inputs are opt-in with safe defaults (false / "" / []), so the change is additive for existing callers.

Cross-repo consumers found

The following repos call jupitermoney/github-workflows/.github/workflows/jvm-post-merge.yml@main or jvm-pre-merge.yml@main directly and inherit the updated workflow contract on every merge to main:

Repo File Notes
sip-engine .github/workflows/jvm-post-merge.yml:11 Calls post-merge with java_version: "21" — unaffected unless s3_deploy is passed
sip-engine .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
cardboard .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
plutus .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
nbfc .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
navigator .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
report-processing-service .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
report-processing-consumer .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
card-mandates .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
ticker .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
inv-vault .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
upi-merchant .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
commerce-marketplace .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected
galactus .github/workflows/jvm-pre-merge.yml:11 Calls pre-merge — unaffected

Additionally, ppi-report-processing-consumer currently has its own bespoke S3/Lambda CI (publish-image.yml uploading to ppi-staging-ppi-staging-lambda-pipeline) — it is a candidate future adopter of this new s3_deploy path, but currently doesn't call the shared workflow.

⚠️ One potential bug to flag: In .github/actions/s3-deploy/action.yml, the gradle-setup step passes java-version: ${{ inputs.java_version }} but the input is declared as java-version (hyphen), while the jvm-post-merge.yml s3-deploy job passes it as java-version: ${{ inputs.java_version }} — fine. However the gradle-setup composite action call uses ${{ inputs.java_version }} (underscore) which may be a variable name mismatch — worth verifying that gradle-setup expects java-version (hyphen) and that inputs.java_version vs inputs.java-version resolves correctly inside the composite action context.

Risk assessment

LOW — All three new inputs default to false/""/[], so every existing consumer calling the shared workflows without these inputs will continue to take the ECR path unchanged. The new s3-deploy job branch is entirely opt-in. The only consumers at risk would be new repos that explicitly set s3_deploy: true (none yet visible in indexed repos), and the potential java-version/java_version input name inconsistency noted above.

Caveats

  • I only see indexed repos (~261 of ~376 in jupitermoney). Some consumers may live in unindexed repos (e.g. *-prod.internal, claude-plugins, github-metadata).
  • I did NOT build, run tests, or validate at runtime.
  • This is a cross-repo review only — for intra-repo correctness use Cursor / GitHub Copilot / human review.

Triggered by Tanmay Agarwal (U0848K3SQEA) via /jarvis review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants