From 125bbd31974a222b21f3081d1f950f8a7befb1ee Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Thu, 11 Nov 2021 12:26:48 +0100 Subject: [PATCH 01/26] [JEP-401] Customizable header proposal --- jep/401/README.adoc | 268 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 268 insertions(+) create mode 100644 jep/401/README.adoc diff --git a/jep/401/README.adoc b/jep/401/README.adoc new file mode 100644 index 00000000..8d7fcdb5 --- /dev/null +++ b/jep/401/README.adoc @@ -0,0 +1,268 @@ += JEP-401: Customizable Jenkins header +:toc: preamble +:toclevels: 3 +ifdef::env-github[] +:tip-caption: :bulb: +:note-caption: :information_source: +:important-caption: :heavy_exclamation_mark: +:caution-caption: :fire: +:warning-caption: :warning: +endif::[] + +.Metadata +[cols="2"] +|=== +| JEP +| 401 + +| Title +| Customizable Jenkins header + +| Sponsor +| link:https://github.com/imonteroperez[Ildefonso Montero] + +// Use the script `set-jep-status ` to update the status. +| Status +| Draft :speech_balloon: + +| Type +| Standards + +| Created +| 2021-11-28 + +// +// +// Uncomment if there is an associated placeholder JIRA issue. +//| JIRA +//| :bulb: link:https://issues.jenkins-ci.org/browse/JENKINS-nnnnn[JENKINS-nnnnn] :bulb: +// +// +// Uncomment if there will be a BDFL delegate for this JEP. +//| BDFL-Delegate +//| :bulb: Link to github user page :bulb: +// +// +// Uncomment if discussion will occur in forum other than jenkinsci-dev@ mailing list. +//| Discussions-To +//| :bulb: Link to where discussion and final status announcement will occur :bulb: +// +// +// Uncomment if this JEP depends on one or more other JEPs. +//| Requires +//| :bulb: JEP-NUMBER, JEP-NUMBER... :bulb: +// +// +// Uncomment and fill if this JEP is rendered obsolete by a later JEP +//| Superseded-By +//| :bulb: JEP-NUMBER :bulb: +// +// +// Uncomment when this JEP status is set to Accepted, Rejected or Withdrawn. +//| Resolution +//| :bulb: Link to relevant post in the jenkinsci-dev@ mailing list archives :bulb: + +|=== + +== Abstract + +Jenkins does not provide a customization mechanism for header. + +Unique existing approach based on the https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin] has a limited capabilities (see Reasoning section for more details). + +It makes it difficult: + +* to provide branding capabilities to include custom logos, styles or other elements, and +* to make feasible to rewrite some functionality or include additional one to some elements like the search bar. + +Sometimes, this limited customization capabilities implies a barrier on Jenkins adoption inside enterprises. + +Having the ability to customize the header (not only from the UI POV) will help to avoid that situation. + +This proposal provides a customization mechanism for a better integration that also reduces current tech debt. + +== Specification + +The main aspect of the change is the introduction of a new extension point `Header` as an interface (or an abstract class) that provides capabilities to render a specific header and a default implementation of that, named `JenkinsHeader` that is enabled by default always. + +All headers will provide a prioritization technique, via https://javadoc.jenkins.io/hudson/Extension.html[ordinal] though the `Extension` annotation, that will help to override, based on its value, which header will be rendered. Default one will provide a `Integer.MIN_VALUE` value. + +On `pageHeader.jelly` file we will perform a refactor to make it more modular providing different sections: + +* `preHeader.jelly`: that will provide the content of the elements that want to be provided/rendered before header div is rendered on the browser +* `headerContent.jelly`: that will provide the content of the header div +* `postHeader.jelly`: that will provide the content of the elements that want to be provided/rendered after the header div is rendered on the browser + +Thus, `pageHeader.jelly` should look like: + +```xml + + + + + + + +``` + +The `get()` method is provided in the default implementation `jenkins.views.JenkinsHeader` and it will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value). In case two or more headers are provided via external plugins with the same priority (and max), then the default one will be provided and a warning message would be provided on the logs. + +In terms of simplicity, proposal aims to use only one extension to do a full replacement of the header. As a follow up, any given implementation of the extension can provide custom extension points for further or more granular customization capabilities. + +=== Extensibility + +Jenkins users will be able to interact with the `Header` extension as follows in case want to customize the header of their instances by creating a custom plugin. For that purpose, they need just to extend `Header` and provide an implementation of the priority method using the ordinal value to specify desired priority value. + + +== Motivation + +Jenkins uses `pageHeader.jelly` file to specify the content of the header. Although it is valid, if a user wants to modify the header to perform some branding operation, Jenkins header branding capabilities are limited. + +There exist some plugins, like: https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin], that allow Jenkins users to customize some parts like CSS and/or Javascript. On the other hand, if a Jenkins user wants to customize/modify some additional business functionality on some menus and search bar, then there is no approach beyond updating/overriding `pageHeader.jelly` from the Jenkins core, which would be a problem on updating the instance due to conflicts. In addition, if the user wants to customize these behaviors it would be good to have those features as REST endpoints. + +Also, other alternatives has been evaluated as workaround like the https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] with no satisfactory results. See Reasoning section for futher details. + +So, this approach is about providing not only UI capabilities, it is also providing extra functionality and better integration. + +== Reasoning + +Let's consider the following dummy example to illustrate reasoning on why particular design decisions were made and also why current approaches were discarded. + +> A Jenkins user wants to modify its current Jenkins instance header to provide a configurable message (default: `Hello World!`) with the account username of the logged user, as well as two (why not?) search bars + +There exists some options to perform parts of the required actions mentioned above: + +* Update the `pageHeader.jelly` content of his Jenkins instance. Discarded in terms of looking for better mechanisms to customize the instance that does not require to override/update its original source code. +* Use https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin]. It could help us to modify some CSS and Javascript elements, and could be used to reach our goals, but it would be so hacky and will not be able to retrieve programatically the configurable message to be included with the username of the logged user. +* Use https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] to override the content of the header using patches. It would help us to have a repeated search box, but not to have the configurable message due to it only will help for static content. + +Given existing approaches does not fullfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad hoc Jenkins plugin that follows the principles provided on Specification section: + +=== Jenkins core + +* Let’s consider the following definition of the `Header` on: `core/src/main/java/jenkins/views/Header.java` + +``` +package jenkins.views; + +import hudson.ExtensionPoint; + +public interface Header extends ExtensionPoint { + + /** + * Check if the header is enabled. By default it is if installed, + * but the logic is deferred in the plugins. + * @return + */ + boolean isHeaderEnabled(); + +} +``` + +* Let’s consider the following implementation of the Jenkins header on: `core/src/main/java/jenkins/views/JenkinsHeader.java` + +``` +package jenkins.views; + +import hudson.Extension; + +@Extension(ordinal = Integer.MIN_VALUE) +public class JenkinsHeader extends Header { + + @Override + public boolean isHeaderEnabled() { + return true; + } + [...] +} +``` + +* As mentioned before, method `get()` from `JenkinsHeader` will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value) + +``` +[...] +@Restricted(NoExternalUse.class) +@CheckForNull +public static Header get() { + List
headers = ExtensionList.lookup(Header.class).stream() + .filter(header -> header.isHeaderEnabled()) + .collect(Collectors.toList()); + if (headers.size() > 0) { + if (headers.size() > 1) { + LOGGER.warning("More than one configured header. This should not happen. Serving the Jenkins default header and please review"); + } else { + return headers.get(0); + } + } + return new JenkinsHeader(); +} +``` + +* Once we launch Jenkins with the proposed changes on the core, we will obtain the expected/current header working without any issue + +=== Custom UI plugin + +* Create a new plugin following the usual procedure +* Provide an implementation of the custom Header (i.e: `src/main/java/org/jenkinsci/plugins/custom/header/CustomHeader.java`) + +``` +[...] +@Extension(ordinal = 100) +public class CustomHeader extends Header { + + @Override + public boolean isHeaderEnabled() { + // Disable/enable the header based on an ENV var and/or system property + boolean isDisabled = System.getProperty(CustomHeader.class.getName() + ".disable") != null ? + "true".equalsIgnoreCase(System.getProperty(CustomHeader.class.getName() + ".disable")) : + "true".equalsIgnoreCase(System.getenv("CUSTOM_HEADER_DISABLE")); + return !isDisabled; + } +} +``` + +* Provide a method in the custom header to retrieve the label which will be with the username. Current code is just an example, but the label could be obtained from the https://javadoc.jenkins.io/jenkins/model/GlobalConfiguration.html[GlobalConfiguration]. + +``` + public static String getHeaderLabel(){ + // This label content could be retrieved programatically. Not coded in aims of simplicity. + return "Hello World!"; + } +``` + +* Provide the jelly files to override the core ones: `headerContent`, `preHeader` and/or `postHeader`. For that purpose, use the common location convention. For the previous example: `src/main/resources/org/jenkinsci/plugins/custom/header/CustomHeader/`. Retrieve the customizable label to be rendered with the username on the `headerContent` file. + +```xml + + +``` + +* See the sample implementation provided in the Reference Implementation section. + +== Backwards Compatibility + +Existing headers will continue to work as expected + +== Security + +No specific security considerations + +== Infrastructure Requirements + +No impact on the Jenkins project infrastructure + +== Testing + +To write tests specific to the header (also using a patched core via https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] are currently difficult. Proposed solution will solve these issues: if a customized header is an extension in a plugin then having this plugin on your test classpath will suffice to let UI tests run in the expected way, regardless of core provenance. + +== Reference Implementation + +* Proposed changes on Jenkins core: https://github.com/jenkinsci/jenkins/pull/5909 +* Prototype of a https://github.com/imonteroperez/custom-header-plugin[Custom Header plugin]. This plugin is modifying the current Jenkins header including an extra search box (just for clarification purposes). + +== References + +Relevant data + +* jenkins-dev ML threads +* JIRA tickets From 781a489560666583e9623950112ab30a9010eb2d Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Thu, 11 Nov 2021 14:01:54 +0100 Subject: [PATCH 02/26] Fix JEP reference --- jep/401/README.adoc | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/jep/401/README.adoc b/jep/401/README.adoc index 8d7fcdb5..529e5989 100644 --- a/jep/401/README.adoc +++ b/jep/401/README.adoc @@ -1,4 +1,4 @@ -= JEP-401: Customizable Jenkins header += JEP-234: Customizable Jenkins header :toc: preamble :toclevels: 3 ifdef::env-github[] @@ -13,7 +13,7 @@ endif::[] [cols="2"] |=== | JEP -| 401 +| 234 | Title | Customizable Jenkins header @@ -87,21 +87,14 @@ The main aspect of the change is the introduction of a new extension point `Head All headers will provide a prioritization technique, via https://javadoc.jenkins.io/hudson/Extension.html[ordinal] though the `Extension` annotation, that will help to override, based on its value, which header will be rendered. Default one will provide a `Integer.MIN_VALUE` value. -On `pageHeader.jelly` file we will perform a refactor to make it more modular providing different sections: - -* `preHeader.jelly`: that will provide the content of the elements that want to be provided/rendered before header div is rendered on the browser -* `headerContent.jelly`: that will provide the content of the header div -* `postHeader.jelly`: that will provide the content of the elements that want to be provided/rendered after the header div is rendered on the browser - +On `pageHeader.jelly` file we will perform a refactor to make it more modular injecting the `headerContent.jelly` that will provide the content of the header. Thus, `pageHeader.jelly` should look like: ```xml - - ``` @@ -230,7 +223,7 @@ public class CustomHeader extends Header { } ``` -* Provide the jelly files to override the core ones: `headerContent`, `preHeader` and/or `postHeader`. For that purpose, use the common location convention. For the previous example: `src/main/resources/org/jenkinsci/plugins/custom/header/CustomHeader/`. Retrieve the customizable label to be rendered with the username on the `headerContent` file. +* Provide the jelly file to override the `headerContent`. For that purpose, use the common location convention. For the previous example: `src/main/resources/org/jenkinsci/plugins/custom/header/CustomHeader/`. Retrieve the customizable label to be rendered with the username on the `headerContent` file. ```xml @@ -264,5 +257,5 @@ To write tests specific to the header (also using a patched core via https://git Relevant data -* jenkins-dev ML threads -* JIRA tickets +* jenkins-dev: https://groups.google.com/g/jenkinsci-dev/c/1tDvSioCaF0 +* Jenkins UX SIG meeting Nov 24: https://docs.google.com/document/d/1QttPwdimNP_120JukigKsRuBvMr34KZhVfsbgq1HFLM/edit# \ No newline at end of file From da44e56a2837d3d27050931f734c8405a3b93f1e Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Thu, 11 Nov 2021 14:04:52 +0100 Subject: [PATCH 03/26] Relocate --- jep/{401 => 234}/README.adoc | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename jep/{401 => 234}/README.adoc (100%) diff --git a/jep/401/README.adoc b/jep/234/README.adoc similarity index 100% rename from jep/401/README.adoc rename to jep/234/README.adoc From b7a2d3b92f78d9734d719daa64c8e33e01a59eb1 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Fri, 12 Nov 2021 10:51:19 +0100 Subject: [PATCH 04/26] Mention on no API compatibility on backward compatibility section --- jep/234/README.adoc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/jep/234/README.adoc b/jep/234/README.adoc index 529e5989..8ef434d3 100644 --- a/jep/234/README.adoc +++ b/jep/234/README.adoc @@ -234,7 +234,12 @@ public class CustomHeader extends Header { == Backwards Compatibility -Existing headers will continue to work as expected +Given this proposal relies on replacement/injection of the `pageHeader` and `headerContent` and the content of that source relies also on UI elements (CSS identifiers, Javascript, etc.) backward compatibility cannot be guaranteed (as happens with themes - documented as https://www.jenkins.io/doc/book/managing/ui-themes/#themes-support-policy[no API compatibility]). + +To deal with these incompatibilities: + +* Consider to place all your required CSS and Javascript code inside your custom plugins if you are going to do a complete refactor of the header. +* Consider to be up-to-date with the latest sources/updates on the `headerContent` in case you were doing minimal changes through your custom header plugin. == Security From 4ad4a02f3f0ac67c30a6bf1d53ddbe006d72ad6b Mon Sep 17 00:00:00 2001 From: Ildefonso Montero Date: Fri, 12 Nov 2021 17:02:49 +0100 Subject: [PATCH 05/26] Apply suggestions from code review Co-authored-by: Baptiste Mathus --- jep/234/README.adoc | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/jep/234/README.adoc b/jep/234/README.adoc index 8ef434d3..b933e0cb 100644 --- a/jep/234/README.adoc +++ b/jep/234/README.adoc @@ -66,28 +66,34 @@ endif::[] == Abstract -Jenkins does not provide a customization mechanism for header. +Jenkins does not provide a customization mechanism for the header. -Unique existing approach based on the https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin] has a limited capabilities (see Reasoning section for more details). +The unique existing approach based on the https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin] has limited capabilities (see Reasoning section for more details). It makes it difficult: * to provide branding capabilities to include custom logos, styles or other elements, and -* to make feasible to rewrite some functionality or include additional one to some elements like the search bar. +* to customize some functionality or include an additional one to some of the elements, like the search bar. -Sometimes, this limited customization capabilities implies a barrier on Jenkins adoption inside enterprises. +Sometimes, these limited customization capabilities can be a barrier to Jenkins adoption inside enterprises. -Having the ability to customize the header (not only from the UI POV) will help to avoid that situation. +Having the ability to customize the header (not only from the User Interface point of view) will help avoid that situation. -This proposal provides a customization mechanism for a better integration that also reduces current tech debt. +This proposal provides a customization mechanism for a better integration that also reduces current technical debt. == Specification -The main aspect of the change is the introduction of a new extension point `Header` as an interface (or an abstract class) that provides capabilities to render a specific header and a default implementation of that, named `JenkinsHeader` that is enabled by default always. +The main change is the introduction of a new extension point `Header` that provides capabilities to render a specific header. +The current behavior is moved into a default implementation named `JenkinsHeader`. -All headers will provide a prioritization technique, via https://javadoc.jenkins.io/hudson/Extension.html[ordinal] though the `Extension` annotation, that will help to override, based on its value, which header will be rendered. Default one will provide a `Integer.MIN_VALUE` value. +All headers will provide a prioritization technique, via the usual link:https://javadoc.jenkins-ci.org/hudson/Extension.html#ordinal--[ordinal] field of the `Extension` annotation. +The default one will use `Integer.MIN_VALUE` value. +In other words, the default one will be the implementation with the least priority. + + +On link:https://github.com/jenkinsci/jenkins/blob/09f0269e87625491d7d897ba0e878a1f7fa31de4/core/src/main/resources/lib/layout/pageHeader.jelly +[`pageHeader.jelly`], we will make it more modular by injecting a (customizable) `headerContent.jelly` that will provide the content of the header. -On `pageHeader.jelly` file we will perform a refactor to make it more modular injecting the `headerContent.jelly` that will provide the content of the header. Thus, `pageHeader.jelly` should look like: ```xml @@ -111,11 +117,13 @@ Jenkins users will be able to interact with the `Header` extension as follows in Jenkins uses `pageHeader.jelly` file to specify the content of the header. Although it is valid, if a user wants to modify the header to perform some branding operation, Jenkins header branding capabilities are limited. -There exist some plugins, like: https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin], that allow Jenkins users to customize some parts like CSS and/or Javascript. On the other hand, if a Jenkins user wants to customize/modify some additional business functionality on some menus and search bar, then there is no approach beyond updating/overriding `pageHeader.jelly` from the Jenkins core, which would be a problem on updating the instance due to conflicts. In addition, if the user wants to customize these behaviors it would be good to have those features as REST endpoints. +Some plugins, like: https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin], allow Jenkins users to customize some parts like CSS and/or Javascript. +On the other hand, if a Jenkins user wants to customize/modify some additional business functionality on some menus and search bar, then there is no approach beyond updating/overriding `pageHeader.jelly` from the Jenkins core, which would be a problem on updating the instance due to conflicts. +In addition, if the user wants to customize these behaviors it would be good to have those features as REST endpoints. -Also, other alternatives has been evaluated as workaround like the https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] with no satisfactory results. See Reasoning section for futher details. +Also, other alternatives have been evaluated as workaround like the https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] with no satisfactory results. See the _Reasoning_ section for further details. -So, this approach is about providing not only UI capabilities, it is also providing extra functionality and better integration. +So, this approach is not only about providing UI capabilities but also about providing extra functionality and better integration. == Reasoning @@ -123,13 +131,14 @@ Let's consider the following dummy example to illustrate reasoning on why partic > A Jenkins user wants to modify its current Jenkins instance header to provide a configurable message (default: `Hello World!`) with the account username of the logged user, as well as two (why not?) search bars -There exists some options to perform parts of the required actions mentioned above: +Some options exist to perform parts of the required actions mentioned above: -* Update the `pageHeader.jelly` content of his Jenkins instance. Discarded in terms of looking for better mechanisms to customize the instance that does not require to override/update its original source code. +* Update the `pageHeader.jelly` content of their Jenkins instance. + Discarded to not require overriding/updating original Jenkins core source code. * Use https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin]. It could help us to modify some CSS and Javascript elements, and could be used to reach our goals, but it would be so hacky and will not be able to retrieve programatically the configurable message to be included with the username of the logged user. -* Use https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] to override the content of the header using patches. It would help us to have a repeated search box, but not to have the configurable message due to it only will help for static content. +* Use https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] to override the content of the header using patches. It would help us have a second search box, but not to have the configurable message because it will only help for static content. -Given existing approaches does not fullfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad hoc Jenkins plugin that follows the principles provided on Specification section: +Given existing approaches do not fulfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad-hoc Jenkins plugin that follows the principles provided in the Specification section: === Jenkins core From 1ef0b93970d26020baff53210e5dc5b3c1f0eb69 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Fri, 12 Nov 2021 17:05:23 +0100 Subject: [PATCH 06/26] Fix broken link --- jep/234/README.adoc | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/jep/234/README.adoc b/jep/234/README.adoc index b933e0cb..c564e2a2 100644 --- a/jep/234/README.adoc +++ b/jep/234/README.adoc @@ -88,11 +88,10 @@ The current behavior is moved into a default implementation named `JenkinsHeader All headers will provide a prioritization technique, via the usual link:https://javadoc.jenkins-ci.org/hudson/Extension.html#ordinal--[ordinal] field of the `Extension` annotation. The default one will use `Integer.MIN_VALUE` value. -In other words, the default one will be the implementation with the least priority. +In other words, the default one will be the implementation with the least priority. -On link:https://github.com/jenkinsci/jenkins/blob/09f0269e87625491d7d897ba0e878a1f7fa31de4/core/src/main/resources/lib/layout/pageHeader.jelly -[`pageHeader.jelly`], we will make it more modular by injecting a (customizable) `headerContent.jelly` that will provide the content of the header. +On https://github.com/jenkinsci/jenkins/blob/09f0269e87625491d7d897ba0e878a1f7fa31de4/core/src/main/resources/lib/layout/pageHeader.jelly[`pageHeader.jelly`], we will make it more modular by injecting a (customizable) `headerContent.jelly` that will provide the content of the header. Thus, `pageHeader.jelly` should look like: @@ -117,8 +116,8 @@ Jenkins users will be able to interact with the `Header` extension as follows in Jenkins uses `pageHeader.jelly` file to specify the content of the header. Although it is valid, if a user wants to modify the header to perform some branding operation, Jenkins header branding capabilities are limited. -Some plugins, like: https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin], allow Jenkins users to customize some parts like CSS and/or Javascript. -On the other hand, if a Jenkins user wants to customize/modify some additional business functionality on some menus and search bar, then there is no approach beyond updating/overriding `pageHeader.jelly` from the Jenkins core, which would be a problem on updating the instance due to conflicts. +Some plugins, like: https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin], allow Jenkins users to customize some parts like CSS and/or Javascript. +On the other hand, if a Jenkins user wants to customize/modify some additional business functionality on some menus and search bar, then there is no approach beyond updating/overriding `pageHeader.jelly` from the Jenkins core, which would be a problem on updating the instance due to conflicts. In addition, if the user wants to customize these behaviors it would be good to have those features as REST endpoints. Also, other alternatives have been evaluated as workaround like the https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] with no satisfactory results. See the _Reasoning_ section for further details. @@ -133,7 +132,7 @@ Let's consider the following dummy example to illustrate reasoning on why partic Some options exist to perform parts of the required actions mentioned above: -* Update the `pageHeader.jelly` content of their Jenkins instance. +* Update the `pageHeader.jelly` content of their Jenkins instance. Discarded to not require overriding/updating original Jenkins core source code. * Use https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin]. It could help us to modify some CSS and Javascript elements, and could be used to reach our goals, but it would be so hacky and will not be able to retrieve programatically the configurable message to be included with the username of the logged user. * Use https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] to override the content of the header using patches. It would help us have a second search box, but not to have the configurable message because it will only help for static content. From 2f170e611dbe16d74734c9516f603218720d0b7b Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Fri, 12 Nov 2021 17:08:20 +0100 Subject: [PATCH 07/26] Testing source block --- jep/234/README.adoc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/jep/234/README.adoc b/jep/234/README.adoc index c564e2a2..3373902f 100644 --- a/jep/234/README.adoc +++ b/jep/234/README.adoc @@ -143,7 +143,8 @@ Given existing approaches do not fulfill this example, we will explore the alter * Let’s consider the following definition of the `Header` on: `core/src/main/java/jenkins/views/Header.java` -``` +[source,java] +---- package jenkins.views; import hudson.ExtensionPoint; @@ -158,7 +159,7 @@ public interface Header extends ExtensionPoint { boolean isHeaderEnabled(); } -``` +---- * Let’s consider the following implementation of the Jenkins header on: `core/src/main/java/jenkins/views/JenkinsHeader.java` From 080272a6cf84a36a56e13aa21fbe9bd3aab3cb9d Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Fri, 12 Nov 2021 17:09:43 +0100 Subject: [PATCH 08/26] Use source,java block --- jep/234/README.adoc | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/jep/234/README.adoc b/jep/234/README.adoc index 3373902f..31ab6991 100644 --- a/jep/234/README.adoc +++ b/jep/234/README.adoc @@ -163,7 +163,8 @@ public interface Header extends ExtensionPoint { * Let’s consider the following implementation of the Jenkins header on: `core/src/main/java/jenkins/views/JenkinsHeader.java` -``` +[source,java] +---- package jenkins.views; import hudson.Extension; @@ -177,11 +178,12 @@ public class JenkinsHeader extends Header { } [...] } -``` +---- * As mentioned before, method `get()` from `JenkinsHeader` will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value) -``` +[source,java] +---- [...] @Restricted(NoExternalUse.class) @CheckForNull @@ -198,7 +200,7 @@ public static Header get() { } return new JenkinsHeader(); } -``` +---- * Once we launch Jenkins with the proposed changes on the core, we will obtain the expected/current header working without any issue @@ -207,7 +209,8 @@ public static Header get() { * Create a new plugin following the usual procedure * Provide an implementation of the custom Header (i.e: `src/main/java/org/jenkinsci/plugins/custom/header/CustomHeader.java`) -``` +[source,java] +---- [...] @Extension(ordinal = 100) public class CustomHeader extends Header { @@ -221,16 +224,17 @@ public class CustomHeader extends Header { return !isDisabled; } } -``` +---- * Provide a method in the custom header to retrieve the label which will be with the username. Current code is just an example, but the label could be obtained from the https://javadoc.jenkins.io/jenkins/model/GlobalConfiguration.html[GlobalConfiguration]. -``` +[source,java] +---- public static String getHeaderLabel(){ // This label content could be retrieved programatically. Not coded in aims of simplicity. return "Hello World!"; } -``` +---- * Provide the jelly file to override the `headerContent`. For that purpose, use the common location convention. For the previous example: `src/main/resources/org/jenkinsci/plugins/custom/header/CustomHeader/`. Retrieve the customizable label to be rendered with the username on the `headerContent` file. From 6dcf673f99ca6e79460396d916d6725448365f06 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Fri, 12 Nov 2021 17:11:15 +0100 Subject: [PATCH 09/26] Use the right acronym --- jep/234/README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jep/234/README.adoc b/jep/234/README.adoc index 31ab6991..3b675b1b 100644 --- a/jep/234/README.adoc +++ b/jep/234/README.adoc @@ -207,7 +207,7 @@ public static Header get() { === Custom UI plugin * Create a new plugin following the usual procedure -* Provide an implementation of the custom Header (i.e: `src/main/java/org/jenkinsci/plugins/custom/header/CustomHeader.java`) +* Provide an implementation of the custom Header (e.g: `src/main/java/org/jenkinsci/plugins/custom/header/CustomHeader.java`) [source,java] ---- From 0d9f3aa1a39e53cbedf50929f336176fc5dcf88c Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Fri, 12 Nov 2021 17:13:47 +0100 Subject: [PATCH 10/26] Remove complicated/redundant sentence --- jep/234/README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jep/234/README.adoc b/jep/234/README.adoc index 3b675b1b..48e0e236 100644 --- a/jep/234/README.adoc +++ b/jep/234/README.adoc @@ -105,7 +105,7 @@ Thus, `pageHeader.jelly` should look like: The `get()` method is provided in the default implementation `jenkins.views.JenkinsHeader` and it will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value). In case two or more headers are provided via external plugins with the same priority (and max), then the default one will be provided and a warning message would be provided on the logs. -In terms of simplicity, proposal aims to use only one extension to do a full replacement of the header. As a follow up, any given implementation of the extension can provide custom extension points for further or more granular customization capabilities. +In terms of simplicity, proposal aims to use only one extension to do a full replacement of the header. === Extensibility From ba6d169b0e16c46ec3d4f426a4e5387e002b8947 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Fri, 12 Nov 2021 17:17:59 +0100 Subject: [PATCH 11/26] Removed redundant section --- jep/234/README.adoc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/jep/234/README.adoc b/jep/234/README.adoc index 48e0e236..6ed329b2 100644 --- a/jep/234/README.adoc +++ b/jep/234/README.adoc @@ -107,11 +107,6 @@ The `get()` method is provided in the default implementation `jenkins.views.Jenk In terms of simplicity, proposal aims to use only one extension to do a full replacement of the header. -=== Extensibility - -Jenkins users will be able to interact with the `Header` extension as follows in case want to customize the header of their instances by creating a custom plugin. For that purpose, they need just to extend `Header` and provide an implementation of the priority method using the ordinal value to specify desired priority value. - - == Motivation Jenkins uses `pageHeader.jelly` file to specify the content of the header. Although it is valid, if a user wants to modify the header to perform some branding operation, Jenkins header branding capabilities are limited. From d24ed8077f382d81ae5e423fe7e8a07683de81ce Mon Sep 17 00:00:00 2001 From: Ildefonso Montero Date: Mon, 15 Nov 2021 16:35:33 +0100 Subject: [PATCH 12/26] Apply suggestions from code review Co-authored-by: Jesse Glick --- jep/234/README.adoc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jep/234/README.adoc b/jep/234/README.adoc index 6ed329b2..a52cfed9 100644 --- a/jep/234/README.adoc +++ b/jep/234/README.adoc @@ -1,4 +1,4 @@ -= JEP-234: Customizable Jenkins header += JEP-0000: Customizable Jenkins header :toc: preamble :toclevels: 3 ifdef::env-github[] @@ -13,7 +13,7 @@ endif::[] [cols="2"] |=== | JEP -| 234 +| 0000 | Title | Customizable Jenkins header @@ -235,7 +235,7 @@ public class CustomHeader extends Header { ```xml - + ``` * See the sample implementation provided in the Reference Implementation section. From e1d733370cf1bdba8d4591b9190f55999e5e5370 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Tue, 16 Nov 2021 11:07:43 +0100 Subject: [PATCH 13/26] Relocated to 0000 folder --- jep/{234 => 0000}/README.adoc | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename jep/{234 => 0000}/README.adoc (100%) diff --git a/jep/234/README.adoc b/jep/0000/README.adoc similarity index 100% rename from jep/234/README.adoc rename to jep/0000/README.adoc From 955f61a2e45b559b2904882b1ee310860005505d Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Tue, 16 Nov 2021 11:17:02 +0100 Subject: [PATCH 14/26] Update some code changes --- jep/0000/README.adoc | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index a52cfed9..276a7457 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -98,12 +98,12 @@ Thus, `pageHeader.jelly` should look like: ```xml - + ``` -The `get()` method is provided in the default implementation `jenkins.views.JenkinsHeader` and it will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value). In case two or more headers are provided via external plugins with the same priority (and max), then the default one will be provided and a warning message would be provided on the logs. +The `get()` method is provided in the extension point `jenkins.views.Header` and it will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value). In terms of simplicity, proposal aims to use only one extension to do a full replacement of the header. @@ -144,15 +144,16 @@ package jenkins.views; import hudson.ExtensionPoint; -public interface Header extends ExtensionPoint { +public abstract class Header extends ExtensionPoint { /** * Check if the header is enabled. By default it is if installed, * but the logic is deferred in the plugins. * @return */ - boolean isHeaderEnabled(); + boolean isEnabled(); + [...] } ---- @@ -168,33 +169,23 @@ import hudson.Extension; public class JenkinsHeader extends Header { @Override - public boolean isHeaderEnabled() { + public boolean isEnabled() { return true; } [...] } ---- -* As mentioned before, method `get()` from `JenkinsHeader` will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value) +* As mentioned before, method `get()` from `Header` will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value) [source,java] ---- -[...] -@Restricted(NoExternalUse.class) -@CheckForNull -public static Header get() { - List
headers = ExtensionList.lookup(Header.class).stream() - .filter(header -> header.isHeaderEnabled()) - .collect(Collectors.toList()); - if (headers.size() > 0) { - if (headers.size() > 1) { - LOGGER.warning("More than one configured header. This should not happen. Serving the Jenkins default header and please review"); - } else { - return headers.get(0); - } + [...] + @Restricted(NoExternalUse.class) + public static Header get() { + Optional
header = ExtensionList.lookup(Header.class).stream().filter(Header::isEnabled).findFirst(); + return header.orElseGet(() -> new JenkinsHeader()); } - return new JenkinsHeader(); -} ---- * Once we launch Jenkins with the proposed changes on the core, we will obtain the expected/current header working without any issue @@ -211,7 +202,7 @@ public static Header get() { public class CustomHeader extends Header { @Override - public boolean isHeaderEnabled() { + public boolean isEnabled() { // Disable/enable the header based on an ENV var and/or system property boolean isDisabled = System.getProperty(CustomHeader.class.getName() + ".disable") != null ? "true".equalsIgnoreCase(System.getProperty(CustomHeader.class.getName() + ".disable")) : From 2499ba3eeb51fd91e9aab54ef9a58999bd1c1b3c Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Tue, 16 Nov 2021 11:18:28 +0100 Subject: [PATCH 15/26] Reorder content --- jep/0000/README.adoc | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index 276a7457..60e91923 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -157,6 +157,18 @@ public abstract class Header extends ExtensionPoint { } ---- +* As mentioned before, method `get()` from `Header` will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value) + +[source,java] +---- + [...] + @Restricted(NoExternalUse.class) + public static Header get() { + Optional
header = ExtensionList.lookup(Header.class).stream().filter(Header::isEnabled).findFirst(); + return header.orElseGet(() -> new JenkinsHeader()); + } +---- + * Let’s consider the following implementation of the Jenkins header on: `core/src/main/java/jenkins/views/JenkinsHeader.java` [source,java] @@ -176,18 +188,6 @@ public class JenkinsHeader extends Header { } ---- -* As mentioned before, method `get()` from `Header` will retrieve the available headers via `Extension` lookup that are enabled and for those obtained it will provide the one with max priority (ordinal value) - -[source,java] ----- - [...] - @Restricted(NoExternalUse.class) - public static Header get() { - Optional
header = ExtensionList.lookup(Header.class).stream().filter(Header::isEnabled).findFirst(); - return header.orElseGet(() -> new JenkinsHeader()); - } ----- - * Once we launch Jenkins with the proposed changes on the core, we will obtain the expected/current header working without any issue === Custom UI plugin From 6dadb21fe8fa8273e3608e23e5134376f9f24c72 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Tue, 16 Nov 2021 11:20:14 +0100 Subject: [PATCH 16/26] Remove Integer.MIN_VALUE on JenkinsHeader --- jep/0000/README.adoc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index 60e91923..e349465a 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -87,8 +87,7 @@ The main change is the introduction of a new extension point `Header` that provi The current behavior is moved into a default implementation named `JenkinsHeader`. All headers will provide a prioritization technique, via the usual link:https://javadoc.jenkins-ci.org/hudson/Extension.html#ordinal--[ordinal] field of the `Extension` annotation. -The default one will use `Integer.MIN_VALUE` value. -In other words, the default one will be the implementation with the least priority. +The default one will be the implementation with the least priority. On https://github.com/jenkinsci/jenkins/blob/09f0269e87625491d7d897ba0e878a1f7fa31de4/core/src/main/resources/lib/layout/pageHeader.jelly[`pageHeader.jelly`], we will make it more modular by injecting a (customizable) `headerContent.jelly` that will provide the content of the header. @@ -175,9 +174,6 @@ public abstract class Header extends ExtensionPoint { ---- package jenkins.views; -import hudson.Extension; - -@Extension(ordinal = Integer.MIN_VALUE) public class JenkinsHeader extends Header { @Override From 9e62a7c5f03778bd0547bae1df06e3865c7cf812 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Tue, 16 Nov 2021 11:26:56 +0100 Subject: [PATCH 17/26] Setup explicitly that code on Reasoning section may differ from the final one --- jep/0000/README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index e349465a..b5b23b6d 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -131,7 +131,7 @@ Some options exist to perform parts of the required actions mentioned above: * Use https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin]. It could help us to modify some CSS and Javascript elements, and could be used to reach our goals, but it would be so hacky and will not be able to retrieve programatically the configurable message to be included with the username of the logged user. * Use https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] to override the content of the header using patches. It would help us have a second search box, but not to have the configurable message because it will only help for static content. -Given existing approaches do not fulfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad-hoc Jenkins plugin that follows the principles provided in the Specification section: +Given existing approaches do not fulfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad-hoc Jenkins plugin that follows the principles provided in the Specification section. The following source code is provided just for illustrative purposes on reasoning how the approach should looks like and may differ of the final implementation (please refer to Reference Implementation section to see the final proposed changes in source code) === Jenkins core From 6231d1083cd40ca3c4be53195793c848d0577f4a Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Tue, 16 Nov 2021 11:40:57 +0100 Subject: [PATCH 18/26] One-line --- jep/0000/README.adoc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index b5b23b6d..7f53a115 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -131,7 +131,9 @@ Some options exist to perform parts of the required actions mentioned above: * Use https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin]. It could help us to modify some CSS and Javascript elements, and could be used to reach our goals, but it would be so hacky and will not be able to retrieve programatically the configurable message to be included with the username of the logged user. * Use https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] to override the content of the header using patches. It would help us have a second search box, but not to have the configurable message because it will only help for static content. -Given existing approaches do not fulfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad-hoc Jenkins plugin that follows the principles provided in the Specification section. The following source code is provided just for illustrative purposes on reasoning how the approach should looks like and may differ of the final implementation (please refer to Reference Implementation section to see the final proposed changes in source code) +Given existing approaches do not fulfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad-hoc Jenkins plugin that follows the principles provided in the Specification section. + +The following source code is provided just for illustrative purposes on reasoning how the approach should looks like and may differ of the final implementation (please refer to Reference Implementation section to see the final proposed changes in source code) === Jenkins core From 6aea87cee2e5d8c65b9da7cc4ec346669769925a Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Tue, 16 Nov 2021 11:44:03 +0100 Subject: [PATCH 19/26] Explain why source code is there --- jep/0000/README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index 7f53a115..7c70bf2a 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -133,7 +133,7 @@ Some options exist to perform parts of the required actions mentioned above: Given existing approaches do not fulfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad-hoc Jenkins plugin that follows the principles provided in the Specification section. -The following source code is provided just for illustrative purposes on reasoning how the approach should looks like and may differ of the final implementation (please refer to Reference Implementation section to see the final proposed changes in source code) +The following source code is provided just for illustrative purposes on reasoning why particular design decisions were made. It may differ from the final implementation (please go to Reference Implementation section to see the final proposed changes in source code) === Jenkins core From e93d9a8da01316bc45deb11c3ccad415d210c302 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Tue, 16 Nov 2021 15:07:33 +0100 Subject: [PATCH 20/26] Refer to full and partial header on Backward Compatibility --- jep/0000/README.adoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index 7c70bf2a..747007ac 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -238,6 +238,10 @@ To deal with these incompatibilities: * Consider to place all your required CSS and Javascript code inside your custom plugins if you are going to do a complete refactor of the header. * Consider to be up-to-date with the latest sources/updates on the `headerContent` in case you were doing minimal changes through your custom header plugin. +For this two scenarios, there are two specific headers, `FullHeader` which is going to be totally independent and will not rely on references to core resources such as images, CSS elements, etc. and `PartialHeader` which is used to perform minimal changes and relies on core resources references. + +Compatibility check for `PartialHeader` will be based on evaluating the field `compatibilityHeaderVersion`. When an incompatible change is made in the header (like the search form API), compatibility header version should be increased. See Reference Implementation for futher details. + == Security No specific security considerations From cc194ad3c5ae36f5d8e64a1d615db383a60ecbe7 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Fri, 19 Nov 2021 16:54:10 +0100 Subject: [PATCH 21/26] Some Jesse's comment/feedback --- jep/0000/README.adoc | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index 747007ac..01926480 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -66,29 +66,13 @@ endif::[] == Abstract -Jenkins does not provide a customization mechanism for the header. - -The unique existing approach based on the https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin] has limited capabilities (see Reasoning section for more details). - -It makes it difficult: - -* to provide branding capabilities to include custom logos, styles or other elements, and -* to customize some functionality or include an additional one to some of the elements, like the search bar. - -Sometimes, these limited customization capabilities can be a barrier to Jenkins adoption inside enterprises. - -Having the ability to customize the header (not only from the User Interface point of view) will help avoid that situation. - -This proposal provides a customization mechanism for a better integration that also reduces current technical debt. +Jenkins does not provide a customization mechanism for the header. This proposal provides a customization mechanism for a better integration that also reduces current technical debt. == Specification The main change is the introduction of a new extension point `Header` that provides capabilities to render a specific header. -The current behavior is moved into a default implementation named `JenkinsHeader`. All headers will provide a prioritization technique, via the usual link:https://javadoc.jenkins-ci.org/hudson/Extension.html#ordinal--[ordinal] field of the `Extension` annotation. -The default one will be the implementation with the least priority. - On https://github.com/jenkinsci/jenkins/blob/09f0269e87625491d7d897ba0e878a1f7fa31de4/core/src/main/resources/lib/layout/pageHeader.jelly[`pageHeader.jelly`], we will make it more modular by injecting a (customizable) `headerContent.jelly` that will provide the content of the header. @@ -108,6 +92,21 @@ In terms of simplicity, proposal aims to use only one extension to do a full rep == Motivation +As mentioned before, Jenkins does not provide a customization mechanism for the header. + +The unique existing approach based on the https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin] has limited capabilities (see Reasoning section for more details). + +It makes it difficult: + +* to provide branding capabilities to include custom logos, styles or other elements, and +* to customize some functionality or include an additional one to some of the elements, like the search bar. + +Sometimes, these limited customization capabilities can be a barrier to Jenkins adoption inside enterprises. + +Having the ability to customize the header (not only from the User Interface point of view) will help avoid that situation. + +== Reasoning + Jenkins uses `pageHeader.jelly` file to specify the content of the header. Although it is valid, if a user wants to modify the header to perform some branding operation, Jenkins header branding capabilities are limited. Some plugins, like: https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin], allow Jenkins users to customize some parts like CSS and/or Javascript. @@ -118,18 +117,16 @@ Also, other alternatives have been evaluated as workaround like the https://gith So, this approach is not only about providing UI capabilities but also about providing extra functionality and better integration. -== Reasoning - Let's consider the following dummy example to illustrate reasoning on why particular design decisions were made and also why current approaches were discarded. > A Jenkins user wants to modify its current Jenkins instance header to provide a configurable message (default: `Hello World!`) with the account username of the logged user, as well as two (why not?) search bars Some options exist to perform parts of the required actions mentioned above: -* Update the `pageHeader.jelly` content of their Jenkins instance. +* Update the `pageHeader.jelly` content of their forked Jenkins instance. Discarded to not require overriding/updating original Jenkins core source code. * Use https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin]. It could help us to modify some CSS and Javascript elements, and could be used to reach our goals, but it would be so hacky and will not be able to retrieve programatically the configurable message to be included with the username of the logged user. -* Use https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] to override the content of the header using patches. It would help us have a second search box, but not to have the configurable message because it will only help for static content. +* Use https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] to override the content of the header using patches. It would help us have a second search box, but not to have the configurable message because it will only help for static content. Also, when core resources change, you get a diff that does not apply, and it is hell to recreate. Given existing approaches do not fulfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad-hoc Jenkins plugin that follows the principles provided in the Specification section. From 0a9b76ad56236644ae1ccfae0147fd6d953749e5 Mon Sep 17 00:00:00 2001 From: Ildefonso Montero Date: Mon, 22 Nov 2021 09:20:15 +0100 Subject: [PATCH 22/26] Apply suggestions from code review Co-authored-by: Mark Waite --- jep/0000/README.adoc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index 01926480..1099e21d 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -94,7 +94,7 @@ In terms of simplicity, proposal aims to use only one extension to do a full rep As mentioned before, Jenkins does not provide a customization mechanism for the header. -The unique existing approach based on the https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin] has limited capabilities (see Reasoning section for more details). +The unique existing approach based on the https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin] has limited capabilities (see <> section for more details). It makes it difficult: @@ -113,7 +113,7 @@ Some plugins, like: https://plugins.jenkins.io/simple-theme-plugin/[simple-theme On the other hand, if a Jenkins user wants to customize/modify some additional business functionality on some menus and search bar, then there is no approach beyond updating/overriding `pageHeader.jelly` from the Jenkins core, which would be a problem on updating the instance due to conflicts. In addition, if the user wants to customize these behaviors it would be good to have those features as REST endpoints. -Also, other alternatives have been evaluated as workaround like the https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] with no satisfactory results. See the _Reasoning_ section for further details. +Also, other alternatives have been evaluated as workaround like the https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] with no satisfactory results. So, this approach is not only about providing UI capabilities but also about providing extra functionality and better integration. @@ -128,9 +128,9 @@ Some options exist to perform parts of the required actions mentioned above: * Use https://plugins.jenkins.io/simple-theme-plugin/[simple-theme-plugin]. It could help us to modify some CSS and Javascript elements, and could be used to reach our goals, but it would be so hacky and will not be able to retrieve programatically the configurable message to be included with the username of the logged user. * Use https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] to override the content of the header using patches. It would help us have a second search box, but not to have the configurable message because it will only help for static content. Also, when core resources change, you get a diff that does not apply, and it is hell to recreate. -Given existing approaches do not fulfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad-hoc Jenkins plugin that follows the principles provided in the Specification section. +Given existing approaches do not fulfill this example, we will explore the alternative of a Jenkins user that wants to update the Jenkins header through an ad-hoc Jenkins plugin that follows the principles provided in the <> section. -The following source code is provided just for illustrative purposes on reasoning why particular design decisions were made. It may differ from the final implementation (please go to Reference Implementation section to see the final proposed changes in source code) +The following source code is provided just for illustrative purposes on reasoning why particular design decisions were made. It may differ from the final implementation (please go to <> section to see the final proposed changes in source code) === Jenkins core @@ -224,7 +224,7 @@ public class CustomHeader extends Header { ``` -* See the sample implementation provided in the Reference Implementation section. +* See the sample implementation provided in the <> section. == Backwards Compatibility @@ -237,7 +237,7 @@ To deal with these incompatibilities: For this two scenarios, there are two specific headers, `FullHeader` which is going to be totally independent and will not rely on references to core resources such as images, CSS elements, etc. and `PartialHeader` which is used to perform minimal changes and relies on core resources references. -Compatibility check for `PartialHeader` will be based on evaluating the field `compatibilityHeaderVersion`. When an incompatible change is made in the header (like the search form API), compatibility header version should be increased. See Reference Implementation for futher details. +Compatibility check for `PartialHeader` will be based on evaluating the field `compatibilityHeaderVersion`. When an incompatible change is made in the header (like the search form API), compatibility header version should be increased. See <> for futher details. == Security From b6b353c1adca78cf8f2432a91cc6da49467906b7 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Mon, 22 Nov 2021 09:35:38 +0100 Subject: [PATCH 23/26] Applied one-sentence-per-line --- jep/0000/README.adoc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index 1099e21d..571cd632 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -249,7 +249,9 @@ No impact on the Jenkins project infrastructure == Testing -To write tests specific to the header (also using a patched core via https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] are currently difficult. Proposed solution will solve these issues: if a customized header is an extension in a plugin then having this plugin on your test classpath will suffice to let UI tests run in the expected way, regardless of core provenance. +To write tests specific to the header (also using a patched core via https://github.com/stephenc/diffpatch-maven-plugin[diffpatch-maven-plugin] are currently difficult. + +Proposed solution will solve these issues: if a customized header is an extension in a plugin then having this plugin on your test classpath will suffice to let UI tests run in the expected way, regardless of core provenance. == Reference Implementation From d1f9b7336ae776ac8293a4bb2d8b252493d19255 Mon Sep 17 00:00:00 2001 From: imonteroperez Date: Mon, 22 Nov 2021 09:40:33 +0100 Subject: [PATCH 24/26] Fix reference implementation description --- jep/0000/README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jep/0000/README.adoc b/jep/0000/README.adoc index 571cd632..bd513d95 100644 --- a/jep/0000/README.adoc +++ b/jep/0000/README.adoc @@ -256,7 +256,7 @@ Proposed solution will solve these issues: if a customized header is an extensio == Reference Implementation * Proposed changes on Jenkins core: https://github.com/jenkinsci/jenkins/pull/5909 -* Prototype of a https://github.com/imonteroperez/custom-header-plugin[Custom Header plugin]. This plugin is modifying the current Jenkins header including an extra search box (just for clarification purposes). +* Prototype of a https://github.com/imonteroperez/custom-header-plugin[Custom Header plugin]. This plugin is replacing the current Jenkins header including a customizable message and a redundant search box (just for clarification purposes) using `PartialHeader`. == References From 46cb96cd7d16e08e964013e339e0bc24cb1b021a Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Tue, 23 Nov 2021 09:27:02 -0700 Subject: [PATCH 25/26] Customizable Jenkins header is now JEP-234 --- jep/{0000 => 234}/README.adoc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename jep/{0000 => 234}/README.adoc (99%) diff --git a/jep/0000/README.adoc b/jep/234/README.adoc similarity index 99% rename from jep/0000/README.adoc rename to jep/234/README.adoc index bd513d95..07af1349 100644 --- a/jep/0000/README.adoc +++ b/jep/234/README.adoc @@ -1,4 +1,4 @@ -= JEP-0000: Customizable Jenkins header += JEP-234: Customizable Jenkins header :toc: preamble :toclevels: 3 ifdef::env-github[] @@ -13,7 +13,7 @@ endif::[] [cols="2"] |=== | JEP -| 0000 +| 234 | Title | Customizable Jenkins header @@ -263,4 +263,4 @@ Proposed solution will solve these issues: if a customized header is an extensio Relevant data * jenkins-dev: https://groups.google.com/g/jenkinsci-dev/c/1tDvSioCaF0 -* Jenkins UX SIG meeting Nov 24: https://docs.google.com/document/d/1QttPwdimNP_120JukigKsRuBvMr34KZhVfsbgq1HFLM/edit# \ No newline at end of file +* Jenkins UX SIG meeting Nov 24: https://docs.google.com/document/d/1QttPwdimNP_120JukigKsRuBvMr34KZhVfsbgq1HFLM/edit# From 85b00c338bf611a23903ddc55d2a86436403c8e1 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Tue, 23 Nov 2021 09:31:47 -0700 Subject: [PATCH 26/26] Accept JEP-234 as a draft JEP --- jep/README.adoc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/jep/README.adoc b/jep/README.adoc index 0e12fd4f..d5000735 100644 --- a/jep/README.adoc +++ b/jep/README.adoc @@ -252,6 +252,11 @@ | https://github.com/basil[Basil{nbsp}Crow] | TBD +| Draft{nbsp}:speech_balloon: +| link:234/README.adoc[JEP-234: Customizable Jenkins header] +| link:https://github.com/imonteroperez[Ildefonso{nbsp}Montero] +| TBD + | Accepted{nbsp}:ok_hand: | link:300/README.adoc[JEP-300: Jenkins Evergreen] | link:https://github.com/rtyler[R.{nbsp}Tyler{nbsp}Croy]