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

1787599 - Provide configuration option to disable UUID uniqueness #2585

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -210,6 +210,7 @@ private ConfigProperties() {

public static final String STANDALONE = "candlepin.standalone";
public static final String ENV_CONTENT_FILTERING = "candlepin.environment_content_filtering";
public static final String USE_SYSTEM_UUID_FOR_MATCHING = "candlepin.use_system_uuid_for_matching";

public static final String CONSUMER_SYSTEM_NAME_PATTERN = "candlepin.consumer_system_name_pattern";
public static final String CONSUMER_PERSON_NAME_PATTERN = "candlepin.consumer_person_name_pattern";
Expand Down Expand Up @@ -386,6 +387,7 @@ private ConfigProperties() {
this.put(STANDALONE, "true");

this.put(ENV_CONTENT_FILTERING, "true");
this.put(USE_SYSTEM_UUID_FOR_MATCHING, "true");

// what constitutes a valid consumer name
this.put(CONSUMER_SYSTEM_NAME_PATTERN, "[\\#\\?\\'\\`\\!@{}()\\[\\]\\?&\\w-\\.]+");
Expand Down
Expand Up @@ -621,13 +621,13 @@ protected void populateEntity(Consumer entity, ConsumerDTO dto) {
}

if (dto.getHypervisorId() == null &&
dto.getFact("dmi.system.uuid") != null &&
dto.getFact(Consumer.Facts.SYSTEM_UUID) != null &&
!"true".equals(dto.getFact("virt.is_guest")) &&
entity.getOwnerId() != null) {
HypervisorId hypervisorId = new HypervisorId(
entity,
ownerCurator.findOwnerById(entity.getOwnerId()),
dto.getFact("dmi.system.uuid"));
dto.getFact(Consumer.Facts.SYSTEM_UUID));
entity.setHypervisorId(hypervisorId);
}

Expand Down Expand Up @@ -727,11 +727,12 @@ public ConsumerDTO create(

// fix for duplicate hypervisor/consumer problem
Consumer consumer = null;
if (ownerKey != null && dto.getFact("dmi.system.uuid") != null &&
if (ownerKey != null && config.getBoolean(ConfigProperties.USE_SYSTEM_UUID_FOR_MATCHING) &&
dto.getFact(Consumer.Facts.SYSTEM_UUID) != null &&
!"true".equalsIgnoreCase(dto.getFact("virt.is_guest"))) {
Owner owner = ownerCurator.getByKey(ownerKey);
if (owner != null) {
consumer = consumerCurator.getHypervisor(dto.getFact("dmi.system.uuid"), owner);
consumer = consumerCurator.getHypervisor(dto.getFact(Consumer.Facts.SYSTEM_UUID), owner);
if (consumer != null) {
consumer.setIdCert(generateIdCert(consumer, false));
this.updateConsumer(consumer.getUuid(), dto, principal);
Expand Down
Expand Up @@ -15,6 +15,8 @@
package org.candlepin.service.impl;

import org.candlepin.auth.Principal;
import org.candlepin.common.config.Configuration;
import org.candlepin.config.ConfigProperties;
import org.candlepin.dto.ModelTranslator;
import org.candlepin.dto.api.v1.HypervisorConsumerDTO;
import org.candlepin.dto.api.v1.HypervisorUpdateResultDTO;
Expand Down Expand Up @@ -61,20 +63,22 @@ public class HypervisorUpdateAction {
private ConsumerType hypervisorType;
private SubscriptionServiceAdapter subAdapter;
private ModelTranslator translator;
private Configuration config;

public static final String CREATE = "create";
protected static String prefix = "hypervisor_update_";

@Inject
public HypervisorUpdateAction(ConsumerCurator consumerCurator,
ConsumerTypeCurator consumerTypeCurator, ConsumerResource consumerResource,
SubscriptionServiceAdapter subAdapter, ModelTranslator translator) {
SubscriptionServiceAdapter subAdapter, ModelTranslator translator,
Configuration config) {
this.consumerCurator = consumerCurator;
this.consumerResource = consumerResource;
this.subAdapter = subAdapter;
this.translator = translator;
this.hypervisorType = consumerTypeCurator.getByLabel(
ConsumerTypeEnum.HYPERVISOR.getLabel(), true);
this.hypervisorType = consumerTypeCurator.getByLabel(ConsumerTypeEnum.HYPERVISOR.getLabel(), true);
this.config = config;
}

@Transactional
Expand All @@ -101,10 +105,12 @@ public Result update(
.getHostConsumersMap(owner, hypervisors);
HypervisorUpdateResultDTO result = new HypervisorUpdateResultDTO();
Map<String, Consumer> systemUuidKnownConsumersMap = new HashMap<>();
for (Consumer consumer : hypervisorKnownConsumersMap.getConsumers()) {
if (consumer.hasFact(Consumer.Facts.SYSTEM_UUID)) {
systemUuidKnownConsumersMap.put(
consumer.getFact(Consumer.Facts.SYSTEM_UUID).toLowerCase(), consumer);
if (config.getBoolean(ConfigProperties.USE_SYSTEM_UUID_FOR_MATCHING)) {
for (Consumer consumer : hypervisorKnownConsumersMap.getConsumers()) {
if (consumer.hasFact(Consumer.Facts.SYSTEM_UUID)) {
systemUuidKnownConsumersMap.put(
consumer.getFact(Consumer.Facts.SYSTEM_UUID).toLowerCase(), consumer);
}
}
}

Expand All @@ -113,16 +119,7 @@ public Result update(
Consumer incoming = incomingHosts.get(hypervisorId);
Consumer knownHost = hypervisorKnownConsumersMap.get(hypervisorId);
// HypervisorId might be different in candlepin
if (knownHost == null && incoming.hasFact(Consumer.Facts.SYSTEM_UUID) &&
systemUuidKnownConsumersMap.get(
incoming.getFact(Consumer.Facts.SYSTEM_UUID).toLowerCase()) != null) {
knownHost = systemUuidKnownConsumersMap.get(incoming.getFact(
Consumer.Facts.SYSTEM_UUID).toLowerCase());
if (knownHost != null) {
log.debug("Found a known host by system uuid");
}
}

knownHost = reconcileByUuid(knownHost, incoming, systemUuidKnownConsumersMap);
Consumer reportedOnConsumer = null;

if (knownHost == null) {
Expand Down Expand Up @@ -205,6 +202,21 @@ else if (jobReporterId == null) {
return new Result(result, hypervisorKnownConsumersMap);
}

private Consumer reconcileByUuid(Consumer knownHost, Consumer incoming,
Map<String, Consumer> systemUuidKnownConsumersMap) {
if (knownHost == null && config.getBoolean(ConfigProperties.USE_SYSTEM_UUID_FOR_MATCHING) &&
incoming.hasFact(Consumer.Facts.SYSTEM_UUID) &&
systemUuidKnownConsumersMap.get(
incoming.getFact(Consumer.Facts.SYSTEM_UUID).toLowerCase()) != null) {

knownHost = systemUuidKnownConsumersMap.get(incoming.getFact(
Consumer.Facts.SYSTEM_UUID).toLowerCase());
if (knownHost != null) {
log.debug("Found a known host by system uuid");
}
}
return knownHost;
}
private boolean updateHypervisorId(Consumer consumer, Owner owner, String reporterId,
String hypervisorId) {

Expand Down
Expand Up @@ -1355,7 +1355,7 @@ public void testGetHypervisorConsumerMap() {
public void testGetHypervisorConsumerMapWithFacts() {
String hypervisorId1 = "Hypervisor";
Consumer consumer1 = new Consumer("testConsumer", "testUser", owner, ct);
consumer1.setFact("dmi.system.uuid", "blah");
consumer1.setFact(Consumer.Facts.SYSTEM_UUID, "blah");
HypervisorId hypervisorId = new HypervisorId(hypervisorId1);
hypervisorId.setOwner(owner);
consumer1.setHypervisorId(hypervisorId);
Expand All @@ -1374,7 +1374,7 @@ public void testGetHypervisorConsumerMapWithFactsAndHypervisorId() {
// first consumer set with only the fact, not the hypervisor
String hypervisorId1 = "Hypervisor";
Consumer consumer1 = new Consumer("testConsumer", "testUser", owner, ct);
consumer1.setFact("dmi.system.uuid", "blah");
consumer1.setFact(Consumer.Facts.SYSTEM_UUID, "blah");
consumer1 = consumerCurator.create(consumer1);

// next consumer set with the hypervisor, not the fact
Expand Down
Expand Up @@ -29,6 +29,8 @@
import static org.mockito.Mockito.when;

import org.candlepin.auth.Principal;
import org.candlepin.common.config.Configuration;
import org.candlepin.config.ConfigProperties;
import org.candlepin.dto.ModelTranslator;
import org.candlepin.dto.StandardTranslator;
import org.candlepin.dto.api.v1.ConsumerDTO;
Expand Down Expand Up @@ -84,6 +86,7 @@ public class HypervisorUpdateJobTest extends BaseJobTest {
private I18n i18n;
private SubscriptionServiceAdapter subAdapter;
private HypervisorUpdateAction hypervisorUpdateAction;
private Configuration config;

private ModelTranslator translator;

Expand All @@ -104,6 +107,7 @@ public void init() {
consumerTypeCurator = mock(ConsumerTypeCurator.class);
subAdapter = mock(SubscriptionServiceAdapter.class);
environmentCurator = mock(EnvironmentCurator.class);
config = mock(Configuration.class);
objectMapper = new ObjectMapper();
when(owner.getId()).thenReturn("joe");

Expand All @@ -128,7 +132,7 @@ public void init() {
"}]}";

hypervisorUpdateAction = new HypervisorUpdateAction(consumerCurator, consumerTypeCurator,
consumerResource, subAdapter, translator);
consumerResource, subAdapter, translator, config);
}

@Test
Expand Down Expand Up @@ -246,14 +250,15 @@ public void hypervisorIdIsOverridenDuringHypervisorReportTest() throws JobExecut
vcm.add(hypervisorId, hypervisor);
when(consumerCurator.getHostConsumersMap(eq(owner),
any(HypervisorUpdateJob.HypervisorList.class))).thenReturn(vcm);
when(config.getBoolean(eq(ConfigProperties.USE_SYSTEM_UUID_FOR_MATCHING))).thenReturn(true);

hypervisorJson =
"{\"hypervisors\":" +
"[{" +
"\"name\" : \"hypervisor_999\"," +
"\"hypervisorId\" : {\"hypervisorId\":\"expectedHypervisorId\"}," +
"\"guestIds\" : [{\"guestId\" : \"guestId_1_999\"}]," +
"\"facts\" : {\"dmi.system.uuid\" : \"myUuid\"}" +
"\"facts\" : {\"" + Consumer.Facts.SYSTEM_UUID + "\" : \"myUuid\"}" +
"}]}";

JobDetail detail = HypervisorUpdateJob.forOwner(owner, hypervisorJson, true, principal, null);
Expand All @@ -272,6 +277,47 @@ public void hypervisorIdIsOverridenDuringHypervisorReportTest() throws JobExecut
assertEquals("expectedhypervisorid", updated.getHypervisorId().getHypervisorId());
}

@Test
public void hypervisorMatchOnUuidTurnedOffTest() throws JobExecutionException {
when(ownerCurator.getByKey(eq("joe"))).thenReturn(owner);
when(ownerCurator.findOwnerById(eq("joe"))).thenReturn(owner);
Consumer hypervisor = new Consumer();
hypervisor.setName("hyper-name");
hypervisor.setOwner(owner);
hypervisor.setFact(Consumer.Facts.SYSTEM_UUID, "myUuid");
String hypervisorId = "existing_hypervisor_id";
hypervisor.setHypervisorId(new HypervisorId(hypervisorId));
VirtConsumerMap vcm = new VirtConsumerMap();
vcm.add(hypervisorId, hypervisor);
when(consumerCurator.getHostConsumersMap(eq(owner),
any(HypervisorUpdateJob.HypervisorList.class))).thenReturn(vcm);
when(config.getBoolean(eq(ConfigProperties.USE_SYSTEM_UUID_FOR_MATCHING))).thenReturn(false);

hypervisorJson =
"{\"hypervisors\":" +
"[{" +
"\"name\" : \"hypervisor_999\"," +
"\"hypervisorId\" : {\"hypervisorId\":\"expectedHypervisorId\"}," +
"\"guestIds\" : [{\"guestId\" : \"guestId_1_999\"}]," +
"\"facts\" : {\"" + Consumer.Facts.SYSTEM_UUID + "\" : \"notMyUuid\"}" +
"}]}";

JobDetail detail = HypervisorUpdateJob.forOwner(owner, hypervisorJson, true, principal, null);
JobExecutionContext ctx = mock(JobExecutionContext.class);
when(ctx.getMergedJobDataMap()).thenReturn(detail.getJobDataMap());

HypervisorUpdateJob job = new HypervisorUpdateJob(
ownerCurator, consumerCurator, translator, hypervisorUpdateAction, i18n, objectMapper);
injector.injectMembers(job);
job.execute(ctx);

ArgumentCaptor<Set<Consumer>> updateCaptor = ArgumentCaptor.forClass(Set.class);
verify(consumerCurator, times(2)).bulkUpdate(updateCaptor.capture(), eq(false));

Consumer updated = updateCaptor.getValue().stream().findFirst().orElse(null);
assertEquals("existing_hypervisor_id", updated.getHypervisorId().getHypervisorId());
}

@Test
public void hypervisorUpdateExecCreateNoHypervisorId() throws JobExecutionException {
when(ownerCurator.getByKey(eq("joe"))).thenReturn(owner);
Expand Down
Expand Up @@ -41,6 +41,7 @@ public ConfigForTesting() {
}
public Configuration initConfig() {
Configuration config = new ConfigForTesting();
config.setProperty(ConfigProperties.USE_SYSTEM_UUID_FOR_MATCHING, "true");
return config;
}

Expand Down
Expand Up @@ -258,6 +258,7 @@ public ConfigForTesting() {

public Configuration initConfig() {
Configuration config = new ConfigForTesting();
config.setProperty(ConfigProperties.USE_SYSTEM_UUID_FOR_MATCHING, "true");
return config;
}

Expand Down