search

jenkinsci / amazon-ecs-plugin

Amazon EC2 Container Service Plugin for Jenkins
https://plugins.jenkins.io/amazon-ecs
MIT License
192 stars 227 forks source link

[#317] Pass noproxy list to the client #318

Closed BradyShober closed 6 months ago

BradyShober commented 1 year ago

Respect the Jenkins No Proxy Host configuration. In our environment we have a proxy configuration to allow our instance to hit some external sites, but since we are running our instance in AWS we would like to use the VPC endpoint for interacting with ECS rather than the traffic going over the internet. Without this change the traffic was attempting to go through our proxy despite us having *.amazonaws.com in the no proxy list and was getting blocked.

Fixes #317

Testing done

Built the plugin and applied it to our instance and verified that it was working with *.amazonaws.com in our no proxy list.

### Submitter checklist
- [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [x] Link to relevant issues in GitHub or Jira
- [x] Link to relevant pull requests, esp. upstream and downstream changes
- [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue
Stericson commented 1 year ago

@BradyShober would you mind adding some tests to cover your changes? This will help prevent future accidental regression of your changes.

BradyShober commented 1 year ago

@Stericson I'm having some difficulty in getting tests for this to work due to the need to mock the Jenkins instance. I've found documentation on using PowerMock to do that but I run into a "SEVERE: Failed to load Jenkins.class" error when trying to run tests. Have you experienced this before in anything you have worked on?

Stericson commented 6 months ago

@BradyShober , Many apologies for failing to follow up on this.

I haven't seen the error you've gotten when trying to write the tests.

I had another question while reviewing this again. Can you help me understand exactly why your change was needed?

setNonProxyHosts is, according to AWS documentation, meant to specify additional hosts to be used which should not go through the proxy.

https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/ClientConfiguration.html#setNonProxyHosts-java.lang.String-

Does this seem like a bug within AWS SDK, or a misconfiguration somewhere else? Am I misunderstanding something about how this should work?

BradyShober commented 6 months ago

@Stericson I will try to explain better why this would be needed. In our environment, by default servers are not able to access any endpoint over the Internet without going through a proxy. To get around this for AWS endpoints, we utilize VPC endpoints so we can call them without going through the proxy. Where this is a problem is if we need to configure Jenkins to utilize the proxy to access some other Internet endpoint, we want to be able to use the no proxy host configuration so that traffic to AWS endpoints does not get sent to the proxy and continues to use the VPC endpoints. This plugin is currently not respecting the Jenkins no proxy host configuration which is what this aims to add.

Stericson commented 6 months ago

@BradyShober Thanks! That helped and I see now where I overlooked the key point in the code change.