Feat/Support for local plugin proxy by rockleez · Pull Request #277 · jenkinsci/office-365-connector-plugin

rockleez · 2022-07-14T21:29:49Z

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

Updated global.jelly to support new fields
Added a Proxy class that contains proxy fields
Updated Webook to support new fields and implemented the proxy to be initialized in the constructor

Testing
Unit Tests

All existing tests that were affected have been updated to mock Webhook.DescriptorImpl so that the proxy could be intialized
Added unit tests for Proxy class

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

…tructor

pom.xml

src/main/java/jenkins/plugins/office365connector/HttpWorker.java

src/main/java/jenkins/plugins/office365connector/Proxy.java

src/main/java/jenkins/plugins/office365connector/Webhook.java

rockleez · 2022-07-17T19:22:33Z

@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.

I have Freestyle Job 1 and Pipeline Job 1 (with an additional step to send a message)
Both jobs have the wehbook configured with some default notifications selected
Running the jobs both work as expected and messages are sent to the teams channel
Add proxy config
Running the jobs again, the proxy config is not used except in Pipeline job 1 in the office365ConnectorSend message
Open both jobs and click save without changing anything
Config is now refreshed
- Running the jobs both work as expected and messages are sent to the teams channel this time via the proxy

rockleez · 2022-08-12T08:18:00Z

@damianszczepanik do you have an eta on when this review can be completed and merged into a release. Thanks

damianszczepanik · 2022-08-27T13:00:24Z

Yes, saving updates job

src/main/java/jenkins/plugins/office365connector/HttpWorker.java

src/main/java/jenkins/plugins/office365connector/Proxy.java

src/main/java/jenkins/plugins/office365connector/Webhook.java

src/main/resources/jenkins/plugins/office365connector/Webhook/global.jelly

…okNotifier

damianszczepanik · 2022-11-10T20:24:25Z

src/main/java/jenkins/plugins/office365connector/Webhook.java

+        }
+
+        @DataBoundSetter
+        public void setIp(String ip) {


IP or address?

settled on using the term host

damianszczepanik · 2022-11-10T20:24:51Z

src/main/java/jenkins/plugins/office365connector/Webhook.java

+        }
+
+        @Nonnull
+        public String getPort() {


Reorder and put port together with ip/address

damianszczepanik · 2022-11-10T20:25:42Z

src/main/resources/jenkins/plugins/office365connector/Webhook/config.jelly

            </f:description>

-            <f:entry title="Notify Build Start" field="startNotification">
+            <f:entry title="Notify Build Start" field="startNotification" >


damianszczepanik · 2022-11-10T20:26:12Z

src/main/resources/jenkins/plugins/office365connector/Webhook/global.jelly

                <f:textbox/>
            </f:entry>
+        <f:advanced>
+            <f:section title="Proxy Config">


Configuration

damianszczepanik · 2022-11-10T20:28:14Z

src/main/resources/jenkins/plugins/office365connector/Webhook/help-ip.html

@@ -0,0 +1 @@
+<div align="help">The IP address of the proxy server</div>


Would the proxy.local.dev be valid value It does not look like IP address?

updated the description

damianszczepanik · 2022-11-10T20:31:35Z

src/test/java/jenkins/plugins/office365connector/workflow/WebhookJobPropertyTest.java


 import static org.assertj.core.api.Assertions.assertThat;
-import static org.powermock.api.mockito.PowerMockito.mock;
+import static org.powermock.api.mockito.PowerMockito.*;


Prefer not to use asterix

not really, now this import is redundant

damianszczepanik · 2022-11-10T20:32:10Z

src/test/java/jenkins/plugins/office365connector/workflow/WebhookJobPropertyTest.java

+
+        when(jenkins.getDescriptorOrDie(Webhook.class)).thenReturn(mockDescriptor);
+        when(Jenkins.get()).thenReturn(jenkins);
+    }


empty line between methods

this should be resolved

damianszczepanik · 2022-11-10T20:32:22Z

src/test/java/jenkins/plugins/office365connector/workflow/WebhookJobPropertyTest.java

+        when(jenkins.getDescriptorOrDie(Webhook.class)).thenReturn(mockDescriptor);
+        when(Jenkins.get()).thenReturn(jenkins);
+    }
    @Test


Rebase the code and fix above

damianszczepanik · 2022-12-13T21:36:53Z

Github says that this still has some conflicts

rockleez added 7 commits July 7, 2022 23:07

global.jelly updated with proxy fields

16e2452

Adding plugin specific proxy support for Office365Connector

94cd34b

Fixing tests

eef318e

Added mock for DescriptorImpl to support changes in Webhook.java cons…

bd50cf8

…tructor

Updated screenshot for README.md

2bce0d1

Fixing null check

9ffa565

Fixing null check

b10ce61

damianszczepanik requested changes Jul 16, 2022

View reviewed changes

rockleez and others added 5 commits July 25, 2022 20:45

Added missing exception

b06ef6f

Modified proxyConfigured to avoid a NPE scenario

ed32801

Using temp reference to descriptor

7af49d5

Fixed return condition

ae547e4

reverted plugin version

09bac61

rockleez requested a review from damianszczepanik August 4, 2022 20:47

damianszczepanik requested changes Aug 27, 2022

View reviewed changes

rockleez added 5 commits August 31, 2022 10:54

Set IOException instead of generic Exception

9b4b793

proxyConfigured takes into account empty, blank and null strings

a09c19e

Removed ProxyTest.java

be436e6

Using jenkins ProxyConfigration to handle plugin proxy info

fc028d1

Refactored to reference ProxyConfiguration in Office365ConnectorWebho…

7f5db72

…okNotifier

rockleez requested review from damianszczepanik and deronek and removed request for damianszczepanik and deronek November 7, 2022 21:57

damianszczepanik requested changes Nov 10, 2022

View reviewed changes

renamed ip to host

2b87fb5

Leemalin Moodley added 3 commits December 6, 2022 11:54

fixed whitespace in xml entry

b728aba

Use of term configuration

4dd5245

Updating jelly help documentation

29db0dd

		@@ -0,0 +1 @@
		<div align="help">The IP address of the proxy server</div>

Uh oh!

Conversation

rockleez commented Jul 14, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rockleez commented Jul 17, 2022

Uh oh!

rockleez commented Aug 12, 2022

Uh oh!

damianszczepanik commented Aug 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

damianszczepanik commented Dec 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants