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

allow alternate ec2 endpoint for region population #330

Merged
merged 3 commits into from
Jan 24, 2019
Merged

allow alternate ec2 endpoint for region population #330

merged 3 commits into from
Jan 24, 2019

Conversation

jmcampanini
Copy link
Contributor

@jmcampanini jmcampanini commented Jan 22, 2019

We're running a jenkins instance inside of a govcloud ec2 box that does not have access to ec2.amazonaws.com. Because of that, the population of the region dropdown fails. This PR is an attempt to allow the user to configure the alternate EC2 endpoint that is used to populate the region dropdown. The value is not used anywhere else.

If there is a way to gracefully handle the manual entering of the region in the dropdown instead, that would work as well.

Related commit: e851e59

// tagging @ajlake

@jvz
Copy link
Member

jvz commented Jan 22, 2019

Would you be able to add any tests for this feature?

@jmcampanini
Copy link
Contributor Author

i've never actually built a jenkins plugin or ran tests with jelly UIs, so i'm not sure how that would work. i could look at writing a test for the doFillRegionItems method, would that work?

@jvz
Copy link
Member

jvz commented Jan 22, 2019

Just any sort of test that verifies the feature exists and isn't broken by someone else.

@jmcampanini jmcampanini changed the title Jc/allow alt ec2 endpoint for region pop allow alt ec2 endpoint for region pop Jan 22, 2019
@jmcampanini jmcampanini changed the title allow alt ec2 endpoint for region pop allow alternate ec2 endpoint for region pop Jan 22, 2019
@jmcampanini jmcampanini changed the title allow alternate ec2 endpoint for region pop allow alternate ec2 endpoint for region population Jan 22, 2019
@jmcampanini
Copy link
Contributor Author

jmcampanini commented Jan 23, 2019

i've added a test for the URL construction. let me know if there's anything else i should do

Copy link
Member

@jvz jvz left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for the contribution!

@@ -58,4 +64,18 @@ public void testConfigRoundtrip() throws Exception {
Cloud actual = r.jenkins.clouds.iterator().next();
r.assertEqualBeans(orig, actual, "cloudName,region,useInstanceProfileForCredentials,accessId,privateKey,instanceCap,roleArn,roleSessionName");
}

/**
* Unit tests related to {@link AmazonEC2Cloud}, but do not require a Jenkins instance.
Copy link
Member

Choose a reason for hiding this comment

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

You might be able to use the @WithoutJenkins annotation on the test method without having to make an inner class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when i move the method to the outer class and add @Test and @WithoutJenkins i end up with this error:

[ERROR] Errors: 
[ERROR] hudson.plugins.ec2.AmazonEC2CloudTest.testEC2EndpointURLCreation(hudson.plugins.ec2.AmazonEC2CloudTest)
[ERROR]   Run 1: AmazonEC2CloudTest.testConfigRoundtrip:61 » NullPointer
[ERROR]   Run 2: AmazonEC2CloudTest.testConfigRoundtrip:61 » NullPointer
[ERROR]   Run 3: AmazonEC2CloudTest.testConfigRoundtrip:61 » NullPointer
[ERROR]   Run 4: AmazonEC2CloudTest.testConfigRoundtrip:61 » NullPointer
[ERROR]   Run 5: AmazonEC2CloudTest.testConfigRoundtrip:61 » NullPointer

maybe im doing something wrong there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here's the stacktrace:

-------------------------------------------------------------------------------
Test set: hudson.plugins.ec2.AmazonEC2CloudTest
-------------------------------------------------------------------------------
Tests run: 5, Failures: 0, Errors: 5, Skipped: 0, Time elapsed: 0.348 s <<< FAILURE! - in hudson.plugins.ec2.AmazonEC2CloudTest
testEC2EndpointURLCreation(hudson.plugins.ec2.AmazonEC2CloudTest)  Time elapsed: 0.342 s  <<< ERROR!
java.lang.NullPointerException
	at jenkins.security.ConfidentialStore.get(ConfidentialStore.java:67)
	at jenkins.security.ConfidentialKey.load(ConfidentialKey.java:47)
	at jenkins.security.CryptoConfidentialKey.getKey(CryptoConfidentialKey.java:41)
	at jenkins.security.CryptoConfidentialKey.decrypt(CryptoConfidentialKey.java:134)
	at hudson.util.HistoricalSecrets.decrypt(HistoricalSecrets.java:49)
	at hudson.util.Secret.decrypt(Secret.java:207)
	at hudson.util.Secret.fromString(Secret.java:239)
	at hudson.plugins.ec2.EC2PrivateKey.<init>(EC2PrivateKey.java:50)
	at hudson.plugins.ec2.EC2Cloud.<init>(EC2Cloud.java:177)
	at hudson.plugins.ec2.AmazonEC2Cloud.<init>(AmazonEC2Cloud.java:74)
	at hudson.plugins.ec2.AmazonEC2CloudTest.testConfigRoundtrip(AmazonEC2CloudTest.java:61)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:33)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:272)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:236)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:386)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:323)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:143)

@jmcampanini
Copy link
Contributor Author

@jvz yay, thanks for reviewing it!

what's the process for getting this merged and released?

@jvz
Copy link
Member

jvz commented Jan 23, 2019

We can wait for another review from another EC2 maintainer, or we'll merge this within a few days if there's no negative feedback. Thanks again!

@thoulen
Copy link
Contributor

thoulen commented Jan 24, 2019

I am starting to merge for the release 1.43

@thoulen thoulen merged commit 1f2de63 into jenkinsci:master Jan 24, 2019
@jmcampanini jmcampanini deleted the jc/allow-alt-ec2-endpoint-for-region-pop branch January 24, 2019 14:42
@jmcampanini
Copy link
Contributor Author

yay! looking forward to 1.43 :)

@jmcampanini
Copy link
Contributor Author

is there an ETA on the release of 1.43?

@thoulen
Copy link
Contributor

thoulen commented Jan 29, 2019 via email

@jmcampanini
Copy link
Contributor Author

friendly ping

@jvz
Copy link
Member

jvz commented Feb 6, 2019

@jmcampanini pings might work better in the gitter channel.

@thoulen
Copy link
Contributor

thoulen commented Feb 6, 2019 via email

@bajacondor
Copy link

@thoulen The link https://repo.jenkins-ci.org/snapshots/org/jenkins-ci/plugins/ec2/1.43-SNAPSHOT/
is 404. What's the status of this?

Thank you

@thoulen
Copy link
Contributor

thoulen commented Mar 12, 2019 via email

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.

4 participants