Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add workload identity to hosted deployment #1708

Merged
merged 5 commits into from
Aug 29, 2024
Merged

Conversation

neel-astro
Copy link
Contributor

@neel-astro neel-astro commented Aug 26, 2024

Description

Changes:

  • Add workload identity flag for astro deployment create and astro deployment update commands for dedicated and standard deployments
  • Add workload identity field in the file-based deployment operation, i.e. astro deployment --deployment-file for dedicated and standard deployments
  • Fix update deployment unit tests to make it stateless, until now a lot of state is shared/global, which makes adding any new unit test extremely tedious.

🎟 Issue(s)

Related #1704

🧪 Functional Testing

  • astro deployment create case with workload identity flag
Screenshot 2024-08-23 at 7 42 23 PM Screenshot 2024-08-23 at 7 41 21 PM
  • astro deployment update case with workload identity flag
Screenshot 2024-08-23 at 7 42 29 PM Screenshot 2024-08-23 at 7 41 40 PM
  • astro deployment case for deployments running on azure/gcp
Screenshot 2024-08-23 at 7 42 36 PM Screenshot 2024-08-23 at 7 43 22 PM
  • astro deployment create case using a deployment yaml file
Screenshot 2024-08-23 at 8 12 22 PM Screenshot 2024-08-23 at 8 13 28 PM
  • astro deployment update case using a deployment yaml file
Screenshot 2024-08-23 at 8 11 25 PM Screenshot 2024-08-23 at 8 11 35 PM

📸 Screenshots

Add screenshots to illustrate the validity of these changes.

📋 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Ran make lint before taking out of draft
  • Added/updated applicable tests
  • Tested against Astro-API (if necessary).
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

tickNum = 10
timeoutNum = 180
listLimit = 1000
dedicatedDeploymentRequest = astroplatformcore.UpdateDedicatedDeploymentRequest{}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dropped the global variable, this makes no difference from CLI functionality pov, but it messes up the unit tests, since the global variables are shared

MockResponseInit()
}

func (s *Suite) TearDownSubTest() {
Copy link
Contributor Author

@neel-astro neel-astro Aug 26, 2024

Choose a reason for hiding this comment

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

This fixes the state leak issue between unit tests from #1698, I had to do this bit here because otherwise, the existing unit tests were making it almost impossible to write new unit tests, and it took me 3 hours to write a single update deployment test scenario in this file 🤷

Copy link
Contributor

@kushalmalani kushalmalani left a comment

Choose a reason for hiding this comment

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

LGTM!

@neel-astro neel-astro merged commit 17c71b8 into main Aug 29, 2024
3 checks passed
@neel-astro neel-astro deleted the depl-workload-identity branch August 29, 2024 14:04
neel-astro added a commit that referenced this pull request Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants