Feat/Support for local plugin proxy #277
Conversation
src/main/java/jenkins/plugins/office365connector/HttpWorker.java
Outdated
Show resolved
Hide resolved
|
@damianszczepanik on a similar not I've noticed that the proxy config only takes effect when the job config is opened and saved even with no changes being made. Is this normal behavior due to the architecture of Jenkins? Example.
|
|
@damianszczepanik do you have an eta on when this review can be completed and merged into a release. Thanks |
|
Yes, saving updates job |
src/main/java/jenkins/plugins/office365connector/HttpWorker.java
Outdated
Show resolved
Hide resolved
src/main/resources/jenkins/plugins/office365connector/Webhook/global.jelly
Show resolved
Hide resolved
rockleez
requested review from
damianszczepanik and
deronek
and removed request for
damianszczepanik and
deronek
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setIp(String ip) { |
There was a problem hiding this comment.
settled on using the term host
| } | ||
|
|
||
| @Nonnull | ||
| public String getPort() { |
There was a problem hiding this comment.
Reorder and put port together with ip/address
| @@ -14,7 +14,7 @@ | |||
| This section defines for which build statuses the notification is sent. | |||
| </f:description> | |||
|
|
|||
| <f:entry title="Notify Build Start" field="startNotification"> | |||
| <f:entry title="Notify Build Start" field="startNotification" > | |||
| @@ -7,5 +7,25 @@ | |||
| <f:entry title="Name" field="globalName"> | |||
| <f:textbox/> | |||
| </f:entry> | |||
| <f:advanced> | |||
| <f:section title="Proxy Config"> | |||
| @@ -0,0 +1 @@ | |||
| <div align="help">The IP address of the proxy server</div> | |||
There was a problem hiding this comment.
Would the proxy.local.dev be valid value It does not look like IP address?
There was a problem hiding this comment.
updated the description
| @@ -1,16 +1,19 @@ | |||
| package jenkins.plugins.office365connector.workflow; | |||
|
|
|||
| import static org.assertj.core.api.Assertions.assertThat; | |||
| import static org.powermock.api.mockito.PowerMockito.mock; | |||
| import static org.powermock.api.mockito.PowerMockito.*; | |||
There was a problem hiding this comment.
Prefer not to use asterix
There was a problem hiding this comment.
not really, now this import is redundant
|
|
||
| when(jenkins.getDescriptorOrDie(Webhook.class)).thenReturn(mockDescriptor); | ||
| when(Jenkins.get()).thenReturn(jenkins); | ||
| } |
There was a problem hiding this comment.
empty line between methods
There was a problem hiding this comment.
this should be resolved
|
|
||
| when(jenkins.getDescriptorOrDie(Webhook.class)).thenReturn(mockDescriptor); | ||
| when(Jenkins.get()).thenReturn(jenkins); | ||
| } | ||
| @Test |
There was a problem hiding this comment.
Rebase the code and fix above
|
Github says that this still has some conflicts |
Summary
This PR addresses JENKINS-61989 and implements proxy support on the Office365ConnectorPlugin itself instead of utilizing the Jenkins global proxy config. In my case i would have to add multiple no proxy hosts to achieve the existing stability of the Jenkins server.
Implementation
Testing
Unit Tests
Integration Tests
Tested using mvn:hpi run and also built a snapshot and deployed to a secondary local Jenkins running on linux
Created test jobs for pipeline and freestyle builds
Blocked incoming connections on the firewall
Used Fiddler Classic to act as a proxy and recorded traffic incoming via the proxy and messages appearing in MS Teams.
Regression testing confirmed that without the with the proxy not configured, the proxy configured in the pipeline, and the proxy configured globally, the plugin continues to function as expected.
Please roll this into a new release once approved