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

[JENKINS-60482] Persist the alternate ec2 endpoint #537

Merged

Conversation

MRamonLeon
Copy link
Contributor

@MRamonLeon MRamonLeon commented Nov 23, 2020

See https://issues.jenkins-ci.org/browse/JENKINS-60482

Simplified version of #439

Reasoning:

The field Alternate EC2 endpoint has only one purpose, to fill up the Region field. If you're in an environment where the default endpoint is not reachable, you can specify here the full url to use instead. Like: https://ec2.us-west-1.amazonaws.com. Once the field is populated, the connection and all other operations are performed using this region, so they should go fine.

It's true that every time you enter the configuration page, the Alternate EC2 endpoint is empty, so the Region field never gets populated until you set the endpoint properly. So it causes a lot of frustration and misunderstanding, IMO.

This PR tries to resolve the problem by persisting this field, so every time you reach the configuration page, the field is established, so the region field is populated correctly. This PR is simpler than #439 where we use the Alternate EC2 endpoint as the full new AWS endpoint (IIUC), which I think it's not necessary.

I've recorded a video to show the behavior: https://photos.app.goo.gl/aADg5roUcKbMqLKj8

CC: @jkfernan @GaToRAiD @troymohl

It helps when the default url is forbidden, you don't need to set it every time you access the configure clouds page.
Copy link
Contributor

@res0nance res0nance left a comment

Choose a reason for hiding this comment

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

Looks ok, perhaps you should add some form of test for its persistence, perhaps just extend one of the Casc test?

@MRamonLeon
Copy link
Contributor Author

I've added a new test @res0nance , fine to you?

@res0nance
Copy link
Contributor

I've added a new test @res0nance , fine to you?

Looks good

@MRamonLeon MRamonLeon merged commit 0780e33 into jenkinsci:master Nov 24, 2020
@MRamonLeon MRamonLeon deleted the JENKINS-60482-just-keep-alt-endpoint branch November 24, 2020 14:52
@res0nance res0nance added the enhancement Feature additions or enhancements label Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature additions or enhancements
Projects
None yet
4 participants