Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#759] Prevent user from downgrading a manifest during update operation #784

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[#759] Prevent user from downgrading a manifest during update operation #784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5bccc8
Draft:Update process should check version and not allow reversion th…
parsharma Sep 23, 2024
07e0961
[Issue-759] If available, channel changes replace component updates in
spyrkob Nov 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions integration-tests/src/test/java/org/wildfly/prospero/it/commonapi/UpdateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.wildfly.prospero.it.commonapi;

import org.apache.commons.io.FileUtils;
import org.assertj.core.api.Assertions;
import org.eclipse.aether.DefaultRepositorySystemSession;
import org.eclipse.aether.RepositorySystem;
import org.eclipse.aether.artifact.Artifact;
Expand All @@ -36,6 +37,7 @@
import org.wildfly.channel.Stream;
import org.wildfly.prospero.actions.ApplyCandidateAction;
import org.wildfly.prospero.api.exceptions.MetadataException;
import org.wildfly.prospero.api.exceptions.OperationException;
import org.wildfly.prospero.metadata.ManifestVersionRecord;
import org.wildfly.prospero.actions.UpdateAction;
import org.wildfly.prospero.api.MavenOptions;
Expand Down Expand Up @@ -78,6 +80,9 @@ public class UpdateTest extends WfCoreTestBase {
public void setUp() throws Exception {
super.setUp();
mockRepo = temp.newFolder("repo");

// remove cached manifests between tests
FileUtils.deleteQuietly(localCachePath.resolve(Path.of("test", "channel")).toFile());
}

@After
Expand Down Expand Up @@ -252,6 +257,43 @@ public void candidateFolderHasToBeEmpty() throws Exception {
.message().contains("Can't install the server into a non empty directory");
}

@Test
public void rejectUpdateWhenManifestDowngradeIsDetected() throws Exception {
// deploy manifest file
File manifestFile = new File(MetadataTestUtils.class.getClassLoader().getResource(CHANNEL_BASE_CORE_19).toURI());
deployManifestFile(mockRepo.toURI().toURL(), manifestFile, "1.0.1");

// provision using channel gav
final ProvisioningDefinition provisioningDefinition = defaultWfCoreDefinition()
.setChannelCoordinates(buildConfigWithMockRepo().toPath().toString())
.setOverrideRepositories(Collections.emptyList()) // reset overrides from defaultWfCoreDefinition()
.build();
installation.provision(provisioningDefinition.toProvisioningConfig(),
provisioningDefinition.resolveChannels(CHANNELS_RESOLVER_FACTORY));

Optional<Artifact> wildflyCliArtifact = readArtifactFromManifest("org.wildfly.core", "wildfly-cli");
assertEquals(BASE_VERSION, wildflyCliArtifact.get().getVersion());

// delete the metadata file, so that the lower version of manifest can be resolved in an update
final Path manifestMetadata = mockRepo.toPath().resolve(Path.of("test", "channel", "maven-metadata.xml"));
Files.delete(manifestMetadata);

// update manifest file
final File updatedManifest = upgradeTestArtifactIn(manifestFile);
deployManifestFile(mockRepo.toURI().toURL(), updatedManifest, "1.0.0");


// update installation
Assertions.assertThatThrownBy(()->
new UpdateAction(outputPath, mavenOptions, new AcceptingConsole(), Collections.emptyList())
.performUpdate())
.isInstanceOf(OperationException.class)
.hasMessageContaining("PRSP000276: Unable to perform the update");

wildflyCliArtifact = readArtifactFromManifest("org.wildfly.core", "wildfly-cli");
assertEquals(BASE_VERSION, wildflyCliArtifact.get().getVersion());
}

private File upgradeTestArtifactIn(File manifestFile) throws IOException, MetadataException {
final ChannelManifest manifest = ManifestYamlSupport.parse(manifestFile);
final List<Stream> streams = manifest.getStreams().stream().map(s -> {
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/src/test/java/org/wildfly/prospero/it/commonapi/WfCoreTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public class WfCoreTestBase {

protected static Artifact resolvedUpgradeArtifact;
protected static Artifact resolvedUpgradeClientArtifact;
private static Path localCachePath;
protected static Path localCachePath;
protected Path outputPath;
protected Path manifestPath;
protected ProvisioningAction installation;
Expand Down
13 changes: 13 additions & 0 deletions prospero-cli/src/main/java/org/wildfly/prospero/cli/CliConsole.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.Scanner;

import org.apache.commons.lang3.StringUtils;
import org.wildfly.prospero.api.ChannelVersionChange;
import org.wildfly.prospero.api.Console;
import org.wildfly.prospero.api.ProvisioningProgressEvent;
import org.wildfly.prospero.api.ArtifactChange;
Expand Down Expand Up @@ -174,6 +175,18 @@ public void updatesFound(List<ArtifactChange> artifactUpdates) {
}
}

public void manifestUpdate(List<ChannelVersionChange> manifestChanges) {
if (manifestChanges.isEmpty()) {
println(CliMessages.MESSAGES.unableToChannelVersions());
} else {
println("");
println(CliMessages.MESSAGES.updatededChannelVersionsHeader());
for (ChannelVersionChange change : manifestChanges) {
println(change.shortDescription());
}
}
}

public void printArtifactChanges(List<ArtifactChange> artifactUpdates) {
if (!artifactUpdates.isEmpty()) {
getStdOut().println(CliMessages.MESSAGES.changesFound());
Expand Down
13 changes: 13 additions & 0 deletions prospero-cli/src/main/java/org/wildfly/prospero/cli/CliMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.Set;

import static java.lang.String.format;
import static org.wildfly.prospero.cli.commands.CliConstants.VERBOSE;

public interface CliMessages {

Expand Down Expand Up @@ -722,4 +723,16 @@ default OperationException cancelledByConfilcts() {
bundle.getString("prospero.updates.apply.candidate.cancel_conflicts"),
CliConstants.NO_CONFLICTS_ONLY));
}

default String fullUpdatesOption(String paramName) {
return format(bundle.getString("prospero.updates.full_list_option"), VERBOSE);
}

default String unableToChannelVersions() {
return bundle.getString("prospero.updates.channel_versions_unknown");
}

default String updatededChannelVersionsHeader() {
return bundle.getString("prospero.updates.channel_updates_header");
}
}
50 changes: 39 additions & 11 deletions prospero-cli/src/main/java/org/wildfly/prospero/cli/commands/UpdateCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.wildfly.prospero.cli.commands;

import static org.wildfly.prospero.cli.commands.CliConstants.VERBOSE;

import java.io.IOException;
import java.net.MalformedURLException;
import java.nio.file.Files;
Expand Down Expand Up @@ -136,7 +138,8 @@ private boolean performUpdate(UpdateAction updateAction, boolean yes, CliConsole
Path targetDir = null;
try {
targetDir = Files.createTempDirectory("update-candidate");
if (buildUpdate(updateAction, targetDir, yes, console, () -> console.confirmUpdates())) {

if (buildUpdate(updateAction, targetDir, yes, console, () -> console.confirmUpdates(), verbose)) {
console.println("");
console.buildUpdatesComplete();

Expand Down Expand Up @@ -204,7 +207,7 @@ public Integer call() throws Exception {

try (UpdateAction updateAction = actionFactory.update(installationDir,
mavenOptions, console, repositories)) {
if (buildUpdate(updateAction, candidateDirectory, yes, console, () -> console.confirmBuildUpdates())) {
if (buildUpdate(updateAction, candidateDirectory, yes, console, () -> console.confirmBuildUpdates(), verbose)) {
console.println("");
console.buildUpdatesComplete();
console.println(CliMessages.MESSAGES.updateCandidateGenerated(candidateDirectory));
Expand Down Expand Up @@ -267,7 +270,9 @@ public Integer call() throws Exception {
throw CliMessages.MESSAGES.notCandidate(candidateDir.toAbsolutePath());
}

console.updatesFound(applyCandidateAction.findUpdates().getArtifactUpdates());
final UpdateSet updates = applyCandidateAction.findUpdates();
printUpdates(console, updates, verbose);

final List<FileConflict> conflicts = applyCandidateAction.getConflicts();
FileConflictPrinter.print(conflicts, console);

Expand Down Expand Up @@ -316,8 +321,8 @@ public Integer call() throws Exception {
RepositoryDefinition.from(temporaryRepositories), temporaryFiles);
console.println(CliMessages.MESSAGES.checkUpdatesHeader(installationDir));
try (UpdateAction updateAction = actionFactory.update(installationDir, mavenOptions, console, repositories)) {
final UpdateSet updateSet = updateAction.findUpdates();
console.updatesFound(updateSet.getArtifactUpdates());
final UpdateSet updates = updateAction.findUpdates();
printUpdates(console, updates, verbose);
}

final float totalTime = (System.currentTimeMillis() - startTime) / 1000f;
Expand Down Expand Up @@ -444,18 +449,21 @@ private FeaturePackLocation getFpl(InstallationProfile knownFeaturePack, String
public UpdateCommand(CliConsole console, ActionFactory actionFactory) {
super(console, actionFactory, CliConstants.Commands.UPDATE,
List.of(
new UpdateCommand.PrepareCommand(console, actionFactory),
new UpdateCommand.ApplyCommand(console, actionFactory),
new UpdateCommand.PerformCommand(console, actionFactory),
new UpdateCommand.ListCommand(console, actionFactory),
new PrepareCommand(console, actionFactory),
new ApplyCommand(console, actionFactory),
new PerformCommand(console, actionFactory),
new ListCommand(console, actionFactory),
new SubscribeCommand(console, actionFactory))
);

}

private static boolean buildUpdate(UpdateAction updateAction, Path updateDirectory, boolean yes, CliConsole console, Supplier<Boolean> confirmation) throws OperationException, ProvisioningException {
private static boolean buildUpdate(UpdateAction updateAction, Path updateDirectory,
boolean yes, CliConsole console, Supplier<Boolean> confirmation,
boolean verbose) throws OperationException, ProvisioningException {
final UpdateSet updateSet = updateAction.findUpdates();
printUpdates(console, updateSet, verbose);

console.updatesFound(updateSet.getArtifactUpdates());
if (updateSet.isEmpty()) {
return false;
}
Expand All @@ -469,6 +477,26 @@ private static boolean buildUpdate(UpdateAction updateAction, Path updateDirecto
return true;
}

private static void printUpdates(CliConsole console, UpdateSet updates, boolean verbose) throws OperationException {
if (updates.hasManifestDowngrade()) {
final String summary = String.join(";", updates.getManifestDowngradeDescriptions());
throw ProsperoLogger.ROOT_LOGGER.manifestDowngrade(summary);
}

// only print the full list of components if asked for or if the manifests versions are not complete
if (!updates.isAuthoritativeManifestVersions() || verbose) {
if (!updates.getManifestChanges().isEmpty()) {
console.manifestUpdate(updates.getManifestChanges());
console.println("");
}
console.updatesFound(updates.getArtifactUpdates());
} else {
console.manifestUpdate(updates.getManifestChanges());
console.println("");
console.println(CliMessages.MESSAGES.fullUpdatesOption(VERBOSE));
}
}

public static void verifyInstallationContainsOnlyProspero(Path dir) throws ArgumentParsingException {
verifyDirectoryContainsInstallation(dir);

Expand Down
4 changes: 4 additions & 0 deletions prospero-cli/src/main/resources/UsageMessages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,10 @@ prospero.update.subscribe.conflict.prompt.continue=Copy metadata files.
prospero.update.subscribe.conflict.prompt.cancel=Quit without generating metadata files.
prospero.update.subscribe.meta.exists=Path `%s` contains a server installation provisioned by the %s already.

prospero.updates.full_list_option=To see the full list of component changes, use `%s` option.
prospero.updates.channel_versions_unknown=Unable to discover updated channel versions. Displaying updated components.
prospero.updates.channel_updates_header=Updates summary

prospero.history.no_updates=No changes found
prospero.history.feature_pack.title=Feature Pack
prospero.history.configuration_model.title=configuration model
Expand Down
114 changes: 114 additions & 0 deletions prospero-cli/src/test/java/org/wildfly/prospero/cli/commands/UpdateCommandTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
import org.wildfly.channel.Repository;
import org.wildfly.prospero.ProsperoLogger;
import org.wildfly.prospero.actions.ApplyCandidateAction;
import org.wildfly.prospero.actions.UpdateAction;
import org.wildfly.prospero.api.ArtifactChange;
import org.wildfly.prospero.api.ChannelVersionChange;
import org.wildfly.prospero.api.FileConflict;
import org.wildfly.prospero.api.MavenOptions;
import org.wildfly.prospero.cli.ActionFactory;
Expand Down Expand Up @@ -337,6 +339,118 @@ public void noConflictArgumentHasNoEffect_WhenNoConflictsAreFound() throws Excep
verify(applyCandidateAction).applyUpdate(ApplyCandidateAction.Type.UPDATE);
}

@Test
public void testBuildUpdateIsRejectedIfManifestDowngradeIsDetected() throws Exception {
System.setProperty(UpdateCommand.JBOSS_MODULE_PATH, installationDir.toString());
when(updateAction.findUpdates()).thenReturn(
new UpdateSet(
List.of(change("1.0.0", "1.0.1")),
List.of(new ChannelVersionChange.Builder("test")
.setOldPhysicalVersion("1.0.1")
.setNewPhysicalVersion("1.0.0")
.build()
),
false
)
);
final Path updatePath = tempFolder.newFolder().toPath();

int exitCode = commandLine.execute(CliConstants.Commands.UPDATE, CliConstants.Commands.PREPARE, CliConstants.CANDIDATE_DIR, updatePath.toString(),
CliConstants.DIR, installationDir.toAbsolutePath().toString());

assertEquals(ReturnCodes.PROCESSING_ERROR, exitCode);
Mockito.verify(actionFactory).update(eq(installationDir.toAbsolutePath()), any(), any(), any());
Mockito.verify(updateAction, never()).buildUpdate(updatePath);

// verify the error contains manifest downgrade
assertThat(getErrorOutput())
.contains(ProsperoLogger.ROOT_LOGGER.manifestDowngrade("test: 1.0.1 -> 1.0.0").getMessage());
}

@Test
public void tesListUpdateShowsOnlySummaryIfManifestsAreAuthoritative() throws Exception {
System.setProperty(UpdateCommand.JBOSS_MODULE_PATH, installationDir.toString());
when(updateAction.findUpdates()).thenReturn(
new UpdateSet(
List.of(change("1.0.0", "1.0.1")),
List.of(new ChannelVersionChange.Builder("test")
.setOldPhysicalVersion("1.0.0")
.setNewPhysicalVersion("1.0.1")
.build()
),
true
)
);
final Path updatePath = tempFolder.newFolder().toPath();

int exitCode = commandLine.execute(CliConstants.Commands.UPDATE, CliConstants.Commands.LIST,
CliConstants.DIR, installationDir.toAbsolutePath().toString());

assertEquals(ReturnCodes.SUCCESS, exitCode);
Mockito.verify(actionFactory).update(eq(installationDir.toAbsolutePath()), any(), any(), any());

// verify the error contains manifest downgrade
assertThat(getStandardOutput())
.contains("test: 1.0.0 -> 1.0.1")
.doesNotContain("org.foo:bar");
}

@Test
public void tesListUpdateShowFullUpdatesIfManifestsIsNotAuthoritative() throws Exception {
System.setProperty(UpdateCommand.JBOSS_MODULE_PATH, installationDir.toString());
when(updateAction.findUpdates()).thenReturn(
new UpdateSet(
List.of(change("1.0.0", "1.0.1")),
List.of(new ChannelVersionChange.Builder("test")
.setOldPhysicalVersion("1.0.0")
.setNewPhysicalVersion("1.0.1")
.build()
),
false
)
);
final Path updatePath = tempFolder.newFolder().toPath();

int exitCode = commandLine.execute(CliConstants.Commands.UPDATE, CliConstants.Commands.LIST,
CliConstants.DIR, installationDir.toAbsolutePath().toString());

assertEquals(ReturnCodes.SUCCESS, exitCode);
Mockito.verify(actionFactory).update(eq(installationDir.toAbsolutePath()), any(), any(), any());

// verify the error contains manifest downgrade
assertThat(getStandardOutput())
.contains("test: 1.0.0 -> 1.0.1")
.contains("org.foo:bar");
}

@Test
public void tesListUpdateShowFullUpdatesIfManifestsIsAuthoritativeAndVerboseOption() throws Exception {
System.setProperty(UpdateCommand.JBOSS_MODULE_PATH, installationDir.toString());
when(updateAction.findUpdates()).thenReturn(
new UpdateSet(
List.of(change("1.0.0", "1.0.1")),
List.of(new ChannelVersionChange.Builder("test")
.setOldPhysicalVersion("1.0.0")
.setNewPhysicalVersion("1.0.1")
.build()
),
true
)
);

int exitCode = commandLine.execute(CliConstants.Commands.UPDATE, CliConstants.Commands.LIST,
CliConstants.DIR, installationDir.toAbsolutePath().toString(),
CliConstants.VERBOSE);

assertEquals(ReturnCodes.SUCCESS, exitCode);
Mockito.verify(actionFactory).update(eq(installationDir.toAbsolutePath()), any(), any(), any());

// verify the error contains manifest downgrade
assertThat(getStandardOutput())
.contains("test: 1.0.0 -> 1.0.1")
.contains("org.foo:bar");
}

private ArtifactChange change(String oldVersion, String newVersion) {
return ArtifactChange.updated(new DefaultArtifact("org.foo", "bar", null, oldVersion),
new DefaultArtifact("org.foo", "bar", null, newVersion));
Expand Down
3 changes: 3 additions & 0 deletions prospero-common/src/main/java/org/wildfly/prospero/ProsperoLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.wildfly.prospero.api.exceptions.InvalidUpdateCandidateException;
import org.wildfly.prospero.api.exceptions.MetadataException;
import org.wildfly.prospero.api.exceptions.NoChannelException;
import org.wildfly.prospero.api.exceptions.OperationException;
import org.wildfly.prospero.api.exceptions.ProvisioningRuntimeException;

import java.io.IOException;
Expand Down Expand Up @@ -403,4 +404,6 @@ public interface ProsperoLogger extends BasicLogger {
@Message(id = 275, value = "The candidate at [%s] was not prepared for %s operation.")
InvalidUpdateCandidateException wrongCandidateOperation(Path candidateServer, ApplyCandidateAction.Type operationType);

@Message(id = 276, value = "Unable to perform the update. The resolved update is older than the current version of the server: [%s]")
OperationException manifestDowngrade(String downgradeDescription);
}
Loading
Loading