Skip to content

Commit 7bdb98b

Browse files
authored
Skip security warnings heading if no warnings (#537)
Test highlights that there is ambiguity in the tested code because there are two booleans to represent a single condition. It is possible to have hideWarning == true and showWarning == true, even though we can only do one or the other but not both. Another pull request or another change in this pull request will need to resolve that issue in the most recently merged pull request.
1 parent 4c4a01f commit 7bdb98b

File tree

2 files changed

+45
-2
lines changed

2 files changed

+45
-2
lines changed

plugin-management-library/src/main/java/io/jenkins/tools/pluginmanager/impl/PluginManager.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -459,9 +459,13 @@ public void showSpecificSecurityWarnings(List<Plugin> plugins) {
459459
// NOTE: By default, the plugin installation manager tool will show security warnings.
460460
// see: https://github.com/jenkinsci/plugin-installation-manager-tool/issues/258
461461
if (!cfg.isHideWarnings()) {
462-
logMessage("\nSecurity warnings:");
462+
boolean headingDisplayed = false;
463463
for (Plugin plugin : plugins) {
464464
if (warningExists(plugin)) {
465+
if (!headingDisplayed) {
466+
logMessage("\nSecurity warnings:");
467+
headingDisplayed = true;
468+
}
465469
String pluginName = plugin.getName();
466470
logMessage(plugin.getSecurityWarnings().stream()
467471
.map(warning -> String.format("%s (%s): %s %s %s", pluginName,

plugin-management-library/src/test/java/io/jenkins/tools/pluginmanager/impl/PluginManagerIntegrationTest.java

+40-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.junit.rules.TemporaryFolder;
2727

2828
import static com.github.stefanbirkner.systemlambda.SystemLambda.tapSystemOut;
29+
import static com.github.stefanbirkner.systemlambda.SystemLambda.tapSystemErr;
2930
import static org.assertj.core.api.Assertions.assertThat;
3031
import static org.assertj.core.api.Assertions.assertThatNoException;
3132
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -56,13 +57,29 @@ public interface Configurator {
5657
}
5758

5859
public PluginManager initPluginManager(Configurator configurator) throws IOException {
60+
return initPluginManagerWithSecurityWarning(false, configurator);
61+
}
62+
63+
public PluginManager initPluginManagerWithSecurityWarning(boolean showSecurityWarnings, Configurator configurator) throws IOException {
5964
Config.Builder configBuilder = Config.builder()
6065
.withJenkinsWar(jenkinsWar.getAbsolutePath())
6166
.withPluginDir(pluginsDir)
62-
.withShowAvailableUpdates(true)
6367
.withIsVerbose(true)
6468
.withDoDownload(false)
6569
.withCachePath(cacheDir.toPath());
70+
if (showSecurityWarnings) {
71+
// if showing security warnings, do not show available updates
72+
configBuilder
73+
.withHideWarnings(false)
74+
.withShowWarnings(true)
75+
.withShowAvailableUpdates(false);
76+
} else {
77+
// if showing available updates, do not show security warnings
78+
configBuilder
79+
.withHideWarnings(true)
80+
.withShowWarnings(false)
81+
.withShowAvailableUpdates(true);
82+
}
6683
configurator.configure(configBuilder);
6784
Config config = configBuilder.build();
6885

@@ -133,6 +150,28 @@ public void showAvailableUpdates_shouldNotFailOnUIThemes() throws Exception {
133150
assertThat(output).doesNotContain("uithemes");
134151
}
135152

153+
@Test
154+
public void securityWarningsShownByDefaultTest() throws Exception {
155+
Plugin pluginScriptSecurityWithSecurityWarning = new Plugin("script-security", "1.30", null, null);
156+
PluginManager pluginManager = initPluginManagerWithSecurityWarning(true,
157+
configBuilder -> configBuilder.withPlugins(Arrays.asList(pluginScriptSecurityWithSecurityWarning)));
158+
159+
String output = tapSystemErr(
160+
() -> pluginManager.start(false));
161+
assertThat(output).contains("Security warnings");
162+
}
163+
164+
@Test
165+
public void securityWarningsNotShownTest() throws Exception {
166+
Plugin pluginScriptSecurityWithSecurityWarning = new Plugin("script-security", "1.30", null, null);
167+
PluginManager pluginManager = initPluginManagerWithSecurityWarning(false,
168+
configBuilder -> configBuilder.withPlugins(Arrays.asList(pluginScriptSecurityWithSecurityWarning)));
169+
170+
String output = tapSystemErr(
171+
() -> pluginManager.start(false));
172+
assertThat(output).doesNotContain("Security warnings");
173+
}
174+
136175
@Test
137176
public void findPluginsAndDependenciesTest() throws IOException {
138177
Plugin plugin1 = new Plugin("plugin1", "1.0", null, null);

0 commit comments

Comments
 (0)