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.

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

feat(TCOMP-2475): Migration from higher version not protected for a connector #898

Merged
merged 19 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -566,9 +566,15 @@ public Map<String, String> migrate(final String id, final int version, final Map
.status(Status.NOT_FOUND)
.entity(new ErrorPayload(COMPONENT_MISSING, "Didn't find component " + id))
.build()));
if (version > comp.getVersion() || version == comp.getVersion()) {
if (version > comp.getVersion()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not >=?
If the config version == the version, we shouldn't migrate it, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we call migration if versions ==

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a comment on jira issue concerning studio's migration handling.
Migration fails in that case and the job cannot be opened.
Same behavior observed in slack topic before the revert.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my...
Okay

log.warn("[Component#migrate] Skipping {}#{} configuration migration due to incoming {} > registry {}.",
comp.getParent().getName(), comp.getName(), version, comp.getVersion());
return config;
}
if (comp.getVersion() != version) {
log.info("[Component#migrate] {}#{} registry version {} - incoming: {}.", comp.getParent().getName(),
comp.getName(), comp.getVersion(), version);
}
return ofNullable(componentDao.findById(id))
.orElseThrow(() -> new WebApplicationException(Response
.status(Response.Status.NOT_FOUND)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ public Map<String, String> migrate(final String id, final int version, final Map
final String versionKey = configuration.getMeta().getPath() + ".__version";
final boolean addedVersion = configToMigrate.putIfAbsent(versionKey, Integer.toString(version)) == null;
try {
if (configuration.getVersion() != version) {
log.info("[ConfigurationType#migrate] {}#{} registry: {} - incoming: {}.",
configuration.getKey().getFamily(), configuration.getKey().getConfigName(),
configuration.getVersion(), version);
}
final Map<String, String> migrated = configuration.getMigrationHandler().migrate(version, configToMigrate);
if (addedVersion) {
migrated.remove(versionKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,24 +201,6 @@ void noMigrateWithUpperVersion() {
assertEquals(null, migrated.get("migrated"));
}

@Test
void noMigrateWithEqualVersion() {
final Map<String, String> migrated = base
.path("component/migrate/{id}/{version}")
.resolveTemplate("id", client.getJdbcId())
.resolveTemplate("version", 2)
.request(APPLICATION_JSON_TYPE)
.post(entity(new HashMap<String, String>() {

{
put("going", "nowhere");
}
}, APPLICATION_JSON_TYPE), new GenericType<Map<String, String>>() {
});
assertEquals(1, migrated.size());
assertEquals(null, migrated.get("migrated"));
}

@Test
void searchIcon() {
assertNotNull(base.path("component/icon/custom/{familyId}/{iconKey}")
Expand Down Expand Up @@ -716,7 +698,7 @@ private void assertComponent(final String plugin, final String family, final Str
}

private void assertIndex(final ComponentIndices index) {
assertEquals(12, index.getComponents().size());
assertEquals(13, index.getComponents().size());

final List<ComponentIndex> list = new ArrayList<>(index.getComponents());
list.sort(Comparator.comparing(o -> o.getId().getFamily() + "#" + o.getId().getName()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ void migrateOver8196DefaultByteBuffer() {
}

private void assertIndex(final ConfigTypeNodes index) {
assertEquals(5, index.getNodes().size());
assertEquals(8, index.getNodes().size());
index.getNodes().keySet().forEach(Assertions::assertNotNull); // assert no null ids
// assert there is at least one parent node
assertTrue(index.getNodes().values().stream().anyMatch(n -> n.getParentId() == null));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.stream.Stream;

import javax.inject.Inject;
import javax.json.JsonObject;
import javax.ws.rs.client.WebTarget;

import org.apache.meecrowave.junit5.MonoMeecrowaveConfig;
Expand All @@ -49,8 +48,9 @@ void environment() {
assertTrue(environment.getLastUpdated().compareTo(new Date(0)) > 0);
final Connectors connectors = environment.getConnectors();
assertTrue(("1.2.3").equals(connectors.getVersion()) || ("1.26.0-SNAPSHOT").equals(connectors.getVersion()));
assertEquals("3a507eb7e52c9acd14c247d62bffecdee6493fc08f9cf69f65b941a64fcbf179", connectors.getPluginsHash());
assertEquals("4960c7dbe95b9df086f06ee6057cc57dd3c3d152be25ee71d964db00e6adbd52", connectors.getPluginsHash());
assertEquals(Arrays.asList("another-test-component", "collection-of-object", "component-with-user-jars",
"file-component", "jdbc-component", "the-test-component"), connectors.getPluginsList());
"file-component", "jdbc-component", "migration-component", "the-test-component"),
connectors.getPluginsList());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
/**
* Copyright (C) 2006-2024 Talend Inc. - www.talend.com
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.talend.sdk.component.server.front;

import static javax.ws.rs.core.MediaType.APPLICATION_JSON_TYPE;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.HashMap;
import java.util.Map;

import javax.inject.Inject;
import javax.ws.rs.client.Entity;
import javax.ws.rs.client.WebTarget;

import org.apache.meecrowave.junit5.MonoMeecrowaveConfig;
import org.junit.jupiter.api.Test;
import org.talend.sdk.component.server.front.model.ConfigTypeNodes;
import org.talend.sdk.component.server.test.ComponentClient;
import org.talend.sdk.component.server.test.migration.MigrationDataSet;
import org.talend.sdk.component.server.test.migration.MigrationDataStore;
import org.talend.sdk.component.server.test.migration.MigrationInput;

@MonoMeecrowaveConfig
class MigrationTest {

@Inject
private ComponentClient client;

@Inject
private WebTarget base;

private final String dataStoreId = "bWlncmF0aW9uLWNvbXBvbmVudCNtaWdyYXRpb24jZGF0YXN0b3JlI2RhdGFzdG9yZQ";

private final String dataSetId = "bWlncmF0aW9uLWNvbXBvbmVudCNtaWdyYXRpb24jZGF0YXNldCNkYXRhc2V0";

/**
* component tests
* registry version: 5
*/
@Test
void migrateComponentLower() {
final int incomingVersion = 3;
Map<String, String> conf = new HashMap<>();
conf.put("configuration.inputKey", "keylevel0");
final Map<String, String> migrated = migrateComponent(getInputComponentId(), incomingVersion, conf);
assertEquals("INPUT", migrated.get("level"));
assertEquals(String.valueOf(MigrationInput.Version), migrated.get("currentVersion"));
assertEquals(String.valueOf(incomingVersion), migrated.get("incomingVersion"));
}

@Test
void migrateComponentEqual() {
Map<String, String> conf = new HashMap<>();
conf.put("configuration.inputKey", "keylevel0");
final Map<String, String> migrated = migrateComponent(getInputComponentId(), MigrationInput.Version, conf);
assertEquals("INPUT", migrated.get("level"));
assertEquals(String.valueOf(MigrationInput.Version), migrated.get("currentVersion"));
assertEquals(String.valueOf(MigrationInput.Version), migrated.get("incomingVersion"));
}

@Test
void migrateComponentGreater() {
Map<String, String> conf = new HashMap<>();
conf.put("configuration.inputKey", "keylevel0");
final Map<String, String> migrated = migrateComponent(getInputComponentId(), 6, conf);
assertEquals(conf, migrated);
}

/**
* dataset tests
* registry version: undefined.
*/
@Test
void migrateDataSetLower() {
final int incomingVersion = -1;
Map<String, String> conf = new HashMap<>();
conf.put("configuration.dataSetKey", "keylevel0");
final Map<String, String> migrated = migrateConfigurationType(getDataSetID(), incomingVersion, conf);
assertEquals("DATASET", migrated.get("configuration.level"));
assertEquals(String.valueOf(MigrationDataSet.Version), migrated.get("configuration.currentVersion"));
assertEquals(String.valueOf(incomingVersion), migrated.get("configuration.incomingVersion"));
}

@Test
void migrateDataSetEqual() {
Map<String, String> conf = new HashMap<>();
conf.put("configuration.dataSetKey", "keylevel0");
// Version value not set defaults to 1
final Map<String, String> migrated = migrateConfigurationType(getDataSetID(), 1, conf);
assertEquals(conf, migrated);
}

@Test
void migrateDataSetGreater() {
Map<String, String> conf = new HashMap<>();
conf.put("configuration.dataSetKey", "keylevel0");
final Map<String, String> migrated = migrateConfigurationType(getDataSetID(), 3, conf);
assertEquals(conf, migrated);
}

/**
* datastore tests
* registry version: 2
*/
@Test
void migrateDataStoreLower() {
final int incomingVersion = 1;
Map<String, String> conf = new HashMap<>();
conf.put("configuration.dataStoreKey", "keylevel0");
final Map<String, String> migrated = migrateConfigurationType(getDataStoreID(), incomingVersion, conf);
assertEquals("DATASTORE", migrated.get("configuration.level"));
assertEquals(String.valueOf(MigrationDataStore.Version), migrated.get("configuration.currentVersion"));
assertEquals(String.valueOf(incomingVersion), migrated.get("configuration.incomingVersion"));
}

@Test
void migrateDataStoreEqual() {
Map<String, String> conf = new HashMap<>();
conf.put("configuration.dataStoreKey", "keylevel0");
final Map<String, String> migrated =
migrateConfigurationType(getDataStoreID(), MigrationDataStore.Version, conf);
assertEquals(conf, migrated);
}

@Test
void migrateDataStoreGreater() {
Map<String, String> conf = new HashMap<>();
conf.put("configuration.dataStoreKey", "keylevel0");
final Map<String, String> migrated = migrateConfigurationType(getDataStoreID(), 3, conf);
assertEquals(conf, migrated);
}

/**
* end tests
*/

private Map<String, String> migrateComponent(final String component, final int version,
final Map<String, String> config) {
return callMigrate("component", component, version, config);
}

private Map<String, String> migrateConfigurationType(final String configuration, final int version,
final Map<String, String> config) {
return callMigrate("configurationtype", configuration, version, config);
}

private Map<String, String> callMigrate(final String endpoint, final String id, final int version,
final Map<String, String> config) {
return base
.path(String.format("/%s/migrate/%s/%d", endpoint, id, version))
.request(APPLICATION_JSON_TYPE)
.post(Entity.entity(config, APPLICATION_JSON_TYPE))
.readEntity(Map.class);
}

private String getInputComponentId() {
return client.getComponentId("migration", "Input");
}

private String getDataSetID() {
ConfigTypeNodes index = client.fetchConfigTypeNodes();
return index.getNodes().get(dataSetId).getId();
}

private String getDataStoreID() {
ConfigTypeNodes index = client.fetchConfigTypeNodes();
return index.getNodes().get(dataStoreId).getId();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,11 @@ class ComponentManagerServiceTest {
@Inject
private ComponentActionDao componentActionDao;

public static final String PLUGINS_HASH = "3a507eb7e52c9acd14c247d62bffecdee6493fc08f9cf69f65b941a64fcbf179";
public static final String PLUGINS_HASH = "4960c7dbe95b9df086f06ee6057cc57dd3c3d152be25ee71d964db00e6adbd52";

public static final List<String> PLUGINS_LIST = Arrays.asList("another-test-component", "collection-of-object",
"component-with-user-jars", "file-component", "jdbc-component", "the-test-component");
"component-with-user-jars", "file-component", "jdbc-component", "migration-component",
"the-test-component");

@Test
void deployExistingPlugin() {
Expand Down Expand Up @@ -144,9 +145,9 @@ void undeployNonExistingPlugin() {
@Order(1)
void checkPluginsNotReloaded() throws Exception {
assertEquals("1.2.3", componentManagerService.getConnectors().getVersion());
assertEquals(6, componentManagerService.manager().getContainer().findAll().stream().count());
assertEquals(7, componentManagerService.manager().getContainer().findAll().stream().count());
Thread.sleep(6000);
assertEquals(6, componentManagerService.manager().getContainer().findAll().stream().count());
assertEquals(7, componentManagerService.manager().getContainer().findAll().stream().count());
assertEquals(PLUGINS_HASH, componentManagerService.getConnectors().getPluginsHash());
assertEquals(PLUGINS_LIST, componentManagerService.getConnectors().getPluginsList());
}
Expand All @@ -155,12 +156,12 @@ void checkPluginsNotReloaded() throws Exception {
@Order(10)
void checkPluginsReloaded() throws Exception {
assertEquals("1.2.3", componentManagerService.getConnectors().getVersion());
assertEquals(6, componentManagerService.manager().getContainer().findAll().stream().count());
assertEquals(7, componentManagerService.manager().getContainer().findAll().stream().count());
assertEquals(PLUGINS_HASH, componentManagerService.getConnectors().getPluginsHash());
assertEquals(PLUGINS_LIST, componentManagerService.getConnectors().getPluginsList());
writeVersion("1.26.0-SNAPSHOT");
Thread.sleep(6000);
assertEquals(6, componentManagerService.manager().getContainer().findAll().stream().count());
assertEquals(7, componentManagerService.manager().getContainer().findAll().stream().count());
final String gav = "org.talend.test1:the-test-component:jar:1.2.6:compile";
String pluginID = getPluginId(gav);
assertNotNull(pluginID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import javax.ws.rs.client.WebTarget;

import org.talend.sdk.component.server.front.model.ComponentIndices;
import org.talend.sdk.component.server.front.model.ConfigTypeNodes;

@ApplicationScoped
public class ComponentClient {
Expand All @@ -38,6 +39,17 @@ public ComponentIndices fetchIndex() {
.get(ComponentIndices.class);
}

public ConfigTypeNodes fetchConfigTypeNodes() {
return base
.path("configurationtype/index")
.queryParam("lightPayload", false)
.queryParam("query", "")
.queryParam("lang", "en")
.request(APPLICATION_JSON_TYPE)
.header("Accept-Encoding", "gzip")
.get(ConfigTypeNodes.class);
}

public String getComponentId(final String family, final String component) {
return fetchIndex()
.getComponents()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,12 @@ private String createM2(final String tempDir) {
final String version = "0.0.1";
createComponent(m2, groupId, artifactId, version, generator::createCustomPlugin);
}
{
final String groupId = "org.talend.test";
final String artifactId = "migration-component";
final String version = "0.0.1";
createComponent(m2, groupId, artifactId, version, generator::createMigrationPlugin);
}
if (Boolean.getBoolean("components.server.beam.active")) {
final String groupId = System.getProperty("components.sample.beam.groupId");
final String artifactId = System.getProperty("components.sample.beam.artifactId");
Expand Down Expand Up @@ -386,6 +392,29 @@ private File createCustomPlugin(final File target) {
});
}

public File createMigrationPlugin(final File target) {
return createRepackaging(target, "org/talend/sdk/component/server/test/migration", out -> {
try {
out.putNextEntry(new JarEntry("TALEND-INF/dependencies.txt"));
out.closeEntry();
out.putNextEntry(new JarEntry("org/talend/test/generated/migration_component/Messages.properties"));
new Properties() {

{
put("migration.datastore.migration._displayName", "JDBC DataStore");
put("migration.dataset.migration._displayName", "JDBC DataSet");
put("migration.Database/migration/Standard._category", "DB/Std/Yes");
put("migration.parameters.user.custom._displayName", "My Custom Action");
put("org.talend.test.generated.jdbc_component.JdbcService$I18n.read", "God save the queen");
}
}.store(out, "i18n for the config types");

} catch (final IOException e) {
fail(e.getMessage());
}
});
}

private File createRepackaging(final File target, final String sourcePackage,
final Consumer<JarOutputStream> custom) {
try (final JarOutputStream outputStream = new JarOutputStream(new FileOutputStream(target))) {
Expand Down
Loading
Loading