diff --git a/java/Makefile.docker b/java/Makefile.docker index 1f77681ed088..ff232bc54230 100644 --- a/java/Makefile.docker +++ b/java/Makefile.docker @@ -2,6 +2,7 @@ # # Docker tests variables +EXECUTOR = docker DOCKER_CONTAINER_BASE = systemsmanagement/uyuni/master/docker/containers/uyuni-master DOCKER_REGISTRY = registry.opensuse.org DOCKER_VOLUMES = -v "$(CURDIR)/../:/manager" @@ -14,8 +15,8 @@ DOCKER_ENV = all :: dockerrun_pg dockerpull :: - docker pull $(DOCKER_REGISTRY)/$(DOCKER_CONTAINER_BASE)-pgsql-4eclipse + $(EXECUTOR) pull $(DOCKER_REGISTRY)/$(DOCKER_CONTAINER_BASE)-pgsql-4eclipse dockerrun_pg :: cp buildconf/test/rhn.conf.postgresql-example buildconf/test/rhn.conf - docker run $(DOCKER_COMMON_OPTS) $(DOCKER_PG_PORTS) $(DOCKER_VOLUMES) $(DOCKER_ENV) $(DOCKER_REGISTRY)/$(DOCKER_CONTAINER_BASE)-pgsql-4eclipse + $(EXECUTOR) run $(DOCKER_COMMON_OPTS) $(DOCKER_PG_PORTS) $(DOCKER_VOLUMES) $(DOCKER_ENV) $(DOCKER_REGISTRY)/$(DOCKER_CONTAINER_BASE)-pgsql-4eclipse diff --git a/java/buildconf/ivy/ivy-suse.xml b/java/buildconf/ivy/ivy-suse.xml index c188144edb12..05c1cdd2f442 100644 --- a/java/buildconf/ivy/ivy-suse.xml +++ b/java/buildconf/ivy/ivy-suse.xml @@ -130,7 +130,7 @@ - + diff --git a/java/code/src/com/redhat/rhn/domain/kickstart/KickstartData.java b/java/code/src/com/redhat/rhn/domain/kickstart/KickstartData.java index 417623ddcf49..db3367c7862c 100644 --- a/java/code/src/com/redhat/rhn/domain/kickstart/KickstartData.java +++ b/java/code/src/com/redhat/rhn/domain/kickstart/KickstartData.java @@ -1458,6 +1458,7 @@ public String getDefaultVirtBridge() { * taskomatic. * @return the Profile associated to this ks data */ + public Profile getCobblerObject(User user) { if (StringUtils.isBlank(getCobblerId())) { return null; diff --git a/java/code/src/com/redhat/rhn/domain/server/Server.java b/java/code/src/com/redhat/rhn/domain/server/Server.java index ced882eb9fed..4b3dfdacfed5 100644 --- a/java/code/src/com/redhat/rhn/domain/server/Server.java +++ b/java/code/src/com/redhat/rhn/domain/server/Server.java @@ -727,6 +727,10 @@ public void setSecret(String secretIn) { } /** + * This may be null in some cases: + * - If a server was bootstrapped with Salt and the key was accepted manually via "salt-key". + * - If a server was created by a user and the user was later on deleted. + * * @return Returns the creator. */ public User getCreator() { diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/SystemHandler.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/SystemHandler.java index ac4097f40605..19ee89004ef5 100644 --- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/SystemHandler.java +++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/SystemHandler.java @@ -6585,6 +6585,10 @@ private KickstartData lookupKsData(String label, Org org) { * @apidoc.returntype #return_int_success() */ public int createSystemRecord(User loggedInUser, Integer sid, String ksLabel) { + if (loggedInUser == null) { + throw new FaultException(-2, "loggedInUserNull", "The logged in user was null!"); + } + Server server = null; try { server = SystemManager.lookupByIdAndUser(sid.longValue(), @@ -6600,9 +6604,7 @@ public int createSystemRecord(User loggedInUser, Integer sid, String ksLabel) { } KickstartData ksData = lookupKsData(ksLabel, loggedInUser.getOrg()); - CobblerSystemCreateCommand cmd = new CobblerSystemCreateCommand( - loggedInUser, ksData.getCobblerObject(loggedInUser).getName(), - ksData, server.getName(), loggedInUser.getOrg().getId()); + CobblerSystemCreateCommand cmd = new CobblerSystemCreateCommand(loggedInUser, ksData, server); cmd.store(); return 1; @@ -6637,6 +6639,10 @@ public int createSystemRecord(User loggedInUser, Integer sid, String ksLabel) { */ public int createSystemRecord(User loggedInUser, String systemName, String ksLabel, String kOptions, String comment, List> netDevices) { + if (loggedInUser == null) { + throw new FaultException(-2, "loggedInUserNull", "The logged in user was null!"); + } + // Determine the user and lookup the kickstart profile KickstartData ksData = lookupKsData(ksLabel, loggedInUser.getOrg()); @@ -6646,9 +6652,18 @@ public int createSystemRecord(User loggedInUser, String systemName, String ksLab server.setOrg(loggedInUser.getOrg()); // Create cobbler command - CobblerUnregisteredSystemCreateCommand cmd; - cmd = new CobblerUnregisteredSystemCreateCommand(loggedInUser, server, - ksData.getCobblerObject(loggedInUser).getName()); + org.cobbler.Profile profile = ksData.getCobblerObject(loggedInUser); + if (profile == null) { + throw new FaultException(-2, "ksLabelProfileError", String.format( + "The profile for the ksLabel \"%s\" could not be found!", + ksData.getLabel() + )); + } + CobblerUnregisteredSystemCreateCommand cmd = new CobblerUnregisteredSystemCreateCommand( + loggedInUser, + server, + profile.getName() + ); // Set network device information to the server for (Map map : netDevices) { diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java index 9f43a9d07e24..2661e46cecee 100644 --- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java +++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/system/test/SystemHandlerTest.java @@ -146,6 +146,7 @@ import com.redhat.rhn.manager.entitlement.EntitlementManager; import com.redhat.rhn.manager.errata.cache.ErrataCacheManager; import com.redhat.rhn.manager.formula.FormulaMonitoringManager; +import com.redhat.rhn.manager.kickstart.cobbler.CobblerXMLRPCHelper; import com.redhat.rhn.manager.profile.ProfileManager; import com.redhat.rhn.manager.rhnpackage.test.PackageManagerTest; import com.redhat.rhn.manager.ssm.SsmOperationManager; @@ -178,6 +179,7 @@ import com.suse.manager.xmlrpc.dto.SystemEventDetailsDto; import org.apache.commons.lang3.StringUtils; +import org.cobbler.CobblerConnection; import org.jmock.Expectations; import org.jmock.Mockery; import org.jmock.imposters.ByteBuddyClassImposteriser; @@ -3195,19 +3197,76 @@ public void testCreateSystemProfileNoHwAddress() throws Exception { } } + @Test + public void testCreateSystemRecordRegistered() throws Exception { + // Arrange + CobblerConnection connection = CobblerXMLRPCHelper.getConnection(admin); + connection.invokeMethod("new_profile"); + var profileId = ((LinkedList) connection.invokeMethod("get_profiles")).get(0).get("uid"); + SystemHandler mockedHandler = getMockedHandler(); + KickstartData k = KickstartDataTest.createTestKickstartData(admin.getOrg()); + k.setCobblerId(profileId.toString()); + int systemId = mockedHandler.createSystemProfile( + admin, + "test system", + Collections.singletonMap("hwAddress", "aa:bb:cc:dd:ee:01") + ); + + // Act + int result = mockedHandler.createSystemRecord(admin, systemId, k.getLabel()); + + // Assert + assertEquals(1, result); + } + + @Test + public void testCreateSystemRecordUnregistered() throws Exception { + // Arrange + CobblerConnection connection = CobblerXMLRPCHelper.getConnection(admin); + connection.invokeMethod("new_profile"); + var profileId = ((LinkedList) connection.invokeMethod("get_profiles")).get(0).get("uid"); + SystemHandler mockedHandler = getMockedHandler(); + String systemName = "test system"; + KickstartData k = KickstartDataTest.createTestKickstartData(admin.getOrg()); + k.setCobblerId(profileId.toString()); + + Map netDevices = Map.of( + "name", "dev1", + "ip", "127.0.0.1", + "mac", "00:00:00:00", + "dnsname", "test.com" + ); + // Act + int result = mockedHandler.createSystemRecord( + admin, + systemName, + k.getLabel(), + "", + "", + List.of(netDevices) + ); + + // Assert + assertEquals(1, result); + } + /** * Tests creating a system profile. * @throws Exception if anything goes wrong */ @Test public void testCreateSystemProfile() throws Exception { + // Arrange String hwAddress = "aa:bb:cc:dd:ee:00"; + + // Act int result = getMockedHandler().createSystemProfile(admin, "test system", Collections.singletonMap("hwAddress", hwAddress)); + + // Assert List nics = NetworkInterfaceFactory .lookupNetworkInterfacesByHwAddress(hwAddress) .collect(Collectors.toList()); - assertEquals(1, nics.size()); Server server = nics.get(0).getServer(); assertEquals("test system", server.getName()); diff --git a/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/CobblerSystemCreateCommand.java b/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/CobblerSystemCreateCommand.java index 0aed19a55f44..53e13c98d854 100644 --- a/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/CobblerSystemCreateCommand.java +++ b/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/CobblerSystemCreateCommand.java @@ -47,16 +47,15 @@ import java.util.Optional; /** - * - * Login to Cobbler's XMLRPC API and get a token + * Create a System inside Cobbler via its XML-RPC API. */ public class CobblerSystemCreateCommand extends CobblerCommand { - private static Logger log = LogManager.getLogger(CobblerSystemCreateCommand.class); + private static final Logger LOG = LogManager.getLogger(CobblerSystemCreateCommand.class); private Action scheduledAction; private final Server server; - private String serverName; - private Long orgId; + private final String serverName; + private final Long orgId; private String mediaPath; private String profileName; private String activationKeys; @@ -79,73 +78,73 @@ public class CobblerSystemCreateCommand extends CobblerCommand { private KickstartData ksData; /** - * @param dhcp true if the network type is dhcp - * @param networkInterfaceIn The name of the network interface - * @param useIpv6GatewayIn whether to use ipv6 gateway - * @param ksDistroIn distro to be provisioned + * Private base constructor that contains shared logic between the different constructors. + * + * @param userIn The user that requests the installation. + * @param serverIn The server that is requested to be created inside Cobbler. + * @param serverNameIn The name of the server. + * @param orgIdIn The organisation the server belongs to. + * @param cobblerProfileNameIn The name of the Cobbler Profile that is used for installation. */ - public void setNetworkInfo(boolean dhcp, String networkInterfaceIn, - boolean useIpv6GatewayIn, String ksDistroIn) { - isDhcp = dhcp; - networkInterface = networkInterfaceIn; - useIpv6Gateway = useIpv6GatewayIn; - ksDistro = ksDistroIn; + private CobblerSystemCreateCommand( + User userIn, + Server serverIn, + String serverNameIn, + Long orgIdIn, + String cobblerProfileNameIn + ) { + super(userIn); + this.serverName = serverNameIn; + this.orgId = orgIdIn; + this.server = serverIn; + this.profileName = cobblerProfileNameIn; } /** - * @param doBridge boolean, whether or not to set up a bridge post-install - * @param name string, name of the bridge - * @param slaves string array, nics to use as slaves - * @param options string, bridge options - * @param isBridgeDhcpIn boolean, if the bridge will use dhcp to obtain an ip address - * @param address string, ip address for the bridge (if isDhcp is false) - * @param netmask string, netmask for the bridge (if isDhcp is false) - * @param gateway string, gateway for the bridge (if isDhcp is false) + * Constructor that persists the Cobbler ID in the database after creating it via {@link #store()}. + * + * @param userIn The user that creates the system. + * @param ksDataIn The Kickstart data that should be used for creating the Cobbler System. + * @param serverIn The Server that should be installed. */ - public void setBridgeInfo(boolean doBridge, String name, - List slaves, String options, boolean isBridgeDhcpIn, - String address, String netmask, String gateway) { - setupBridge = doBridge; - bridgeName = name; - bridgeSlaves = slaves; - bridgeOptions = options; - isBridgeDhcp = isBridgeDhcpIn; - bridgeAddress = address; - bridgeNetmask = netmask; - bridgeGateway = gateway; + public CobblerSystemCreateCommand(User userIn, KickstartData ksDataIn, Server serverIn) { + this( + userIn, + serverIn, + serverIn.getName(), + userIn.getOrg().getId(), + ksDataIn.getCobblerObject(userIn).getName() + ); + this.ksData = ksDataIn; } /** * Constructor - * @param userIn who is requesting the sync - * @param serverIn profile we want to create in cobbler - * @param ksDataIn profile to associate with with server. - * @param mediaPathIn mediaPath to override in the server profile. + * + * @param userIn who is requesting the sync + * @param serverIn profile we want to create in cobbler + * @param ksDataIn profile to associate with the server. + * @param mediaPathIn mediaPath to override in the server profile. * @param activationKeysIn to add to the system record. Used when the system - * re-registers to Spacewalk + * re-registers to Spacewalk */ public CobblerSystemCreateCommand(User userIn, Server serverIn, - KickstartData ksDataIn, String mediaPathIn, String activationKeysIn) { - super(userIn); - this.server = serverIn; - this.serverName = serverIn.getName(); - this.orgId = serverIn.getOrgId(); + KickstartData ksDataIn, String mediaPathIn, String activationKeysIn) { + this(userIn, serverIn, serverIn.getName(), serverIn.getOrgId(), ""); this.mediaPath = mediaPathIn; - if (ksDataIn != null) { - profileName = ksDataIn.getCobblerObject(user).getName(); - } - else { + if (ksDataIn == null) { throw new NullPointerException("ksDataIn cant be null"); } + this.profileName = ksDataIn.getCobblerObject(user).getName(); this.activationKeys = activationKeysIn; this.ksData = ksDataIn; } /** - * Constructor to be used for a system outside tthe context + * Constructor to be used for a system outside the context * of actually kickstarting it to a specific profile. * - * @param userIn who is requesting the sync + * @param userIn the user creating the system * @param serverIn profile we want to create in cobbler * @param cobblerProfileName the name of the cobbler profile * to associate with system @@ -153,84 +152,112 @@ public CobblerSystemCreateCommand(User userIn, Server serverIn, */ public CobblerSystemCreateCommand(User userIn, Server serverIn, String cobblerProfileName, KickstartData ksDataIn) { - super(userIn); - this.server = serverIn; - this.serverName = serverIn.getName(); - this.orgId = serverIn.getOrgId(); + this(userIn, serverIn, serverIn.getName(), serverIn.getOrgId(), cobblerProfileName); this.mediaPath = null; - this.profileName = cobblerProfileName; - String keys = ""; String note = "Reactivation key for " + serverName + "."; ActivationKey key = ActivationKeyManager.getInstance(). createNewReActivationKey(UserFactory.findRandomOrgAdmin( - server.getOrg()), server, note); + serverIn.getOrg()), serverIn, note); key.setUsageLimit(1L); - log.debug("created reactivation key: {}", key.getKey()); - keys = key.getKey(); + LOG.debug("created reactivation key: {}", key.getKey()); this.ksData = ksDataIn; - if (this.ksData != null) { - for (Token token : this.ksData.getDefaultRegTokens()) { - ActivationKey keyTmp = ActivationKeyFactory.lookupByToken(token); - if (keyTmp != null) { - keys += "," + keyTmp.getKey(); - } - } - } - this.activationKeys = keys; + this.activationKeys = generateActivationKeys(key.getKey()); } /** * Constructor to be used to create a new system with a cobbler profile. * - * @param userIn the user creating the system + * @param userIn the user creating the system * @param cobblerProfileName the name of the cobbler profile - * to associate with system - * @param ksDataIn the kickstart data to associate the system with - * @param serverNameIn the name of the system to create - * @param orgIdIn the organization ID the system will belong to + * to associate with system + * @param ksDataIn the kickstart data to associate the system with + * @param serverNameIn the name of the system to create + * @param orgIdIn the organization ID the system will belong to */ public CobblerSystemCreateCommand(User userIn, String cobblerProfileName, KickstartData ksDataIn, - String serverNameIn, Long orgIdIn) { - super(userIn); - this.server = null; - this.serverName = serverNameIn; - this.orgId = orgIdIn; + String serverNameIn, Long orgIdIn) { + this(userIn, null, serverNameIn, orgIdIn, cobblerProfileName); this.mediaPath = null; - this.profileName = cobblerProfileName; - String keys = ""; this.ksData = ksDataIn; - if (this.ksData != null) { - for (Token token : this.ksData.getDefaultRegTokens()) { - ActivationKey keyTmp = ActivationKeyFactory.lookupByToken(token); - if (keyTmp != null) { - if (!keys.isBlank()) { - keys += ","; - } - keys += keyTmp.getKey(); - } - } - } - this.activationKeys = keys; + this.activationKeys = generateActivationKeys(""); } /** * Constructor - * @param userIn who is requesting the sync + * + * @param userIn who is requesting the sync * @param serverIn profile we want to create in cobbler - * @param nameIn profile name to associate with with server. + * @param nameIn profile name to associate with server. */ public CobblerSystemCreateCommand(User userIn, Server serverIn, - String nameIn) { - super(userIn); - this.server = serverIn; - this.serverName = serverIn.getName(); - this.orgId = serverIn.getOrgId(); - profileName = nameIn; + String nameIn) { + this(userIn, serverIn, serverIn.getName(), serverIn.getOrgId(), nameIn); + } + + /** + * Generate the activate key string that can be passed to Cobbler's "redhat_management_keys" string. + * + * @param keyIn In case of reactivation of a system you can pass this as a seed to this method. + * @return The activation key or keys. In case no kickstart data is set, this method returns an empty string. + */ + private String generateActivationKeys(String keyIn) { + StringBuilder keys = new StringBuilder(keyIn); + if (this.ksData == null) { + return ""; + } + for (Token token : this.ksData.getDefaultRegTokens()) { + ActivationKey keyTmp = ActivationKeyFactory.lookupByToken(token); + if (keyTmp != null) { + if (!keys.toString().isBlank()) { + keys.append(","); + } + keys.append(keyTmp.getKey()); + } + } + return keys.toString(); + } + + /** + * @param dhcp true if the network type is dhcp + * @param networkInterfaceIn The name of the network interface + * @param useIpv6GatewayIn whether to use ipv6 gateway + * @param ksDistroIn distro to be provisioned + */ + public void setNetworkInfo(boolean dhcp, String networkInterfaceIn, + boolean useIpv6GatewayIn, String ksDistroIn) { + isDhcp = dhcp; + networkInterface = networkInterfaceIn; + useIpv6Gateway = useIpv6GatewayIn; + ksDistro = ksDistroIn; + } + + /** + * @param doBridge boolean, whether or not to set up a bridge post-install + * @param name string, name of the bridge + * @param slaves string array, nics to use as slaves + * @param options string, bridge options + * @param isBridgeDhcpIn boolean, if the bridge will use dhcp to obtain an ip address + * @param address string, ip address for the bridge (if isDhcp is false) + * @param netmask string, netmask for the bridge (if isDhcp is false) + * @param gateway string, gateway for the bridge (if isDhcp is false) + */ + public void setBridgeInfo(boolean doBridge, String name, + List slaves, String options, boolean isBridgeDhcpIn, + String address, String netmask, String gateway) { + setupBridge = doBridge; + bridgeName = name; + bridgeSlaves = slaves; + bridgeOptions = options; + isBridgeDhcp = isBridgeDhcpIn; + bridgeAddress = address; + bridgeNetmask = netmask; + bridgeGateway = gateway; } /** * Store the System to cobbler + * * @return ValidatorError if the store failed. */ @Override @@ -240,27 +267,14 @@ public ValidatorError store() { /** * Store the System to cobbler + * * @param saveCobblerId false if CobblerVirtualSystemCommand is calling, true otherwise * @return ValidatorError if the store failed. */ public ValidatorError store(boolean saveCobblerId) { Profile profile = Profile.lookupByName(getCobblerConnection(), profileName); - // First lookup by MAC addr - SystemRecord rec = null; - if (server != null) { - rec = lookupExisting(server); - if (rec == null) { - // Next try by name - rec = SystemRecord.lookupByName(getCobblerConnection(user), - getCobblerSystemRecordName()); - } - } + SystemRecord rec = getCobblerSystem(profile); - // Else, lets make a new system - if (rec == null) { - rec = SystemRecord.create(getCobblerConnection(), - getCobblerSystemRecordName(), profile); - } if (server != null) { try { processNetworkInterfaces(rec, server); @@ -277,38 +291,77 @@ public ValidatorError store(boolean saveCobblerId) { } rec.enableNetboot(true); rec.setProfile(profile); + rec.setIpv6Autoconfiguration(isDhcp); - if (isDhcp) { - rec.setIpv6Autoconfiguration(true); + processRedHatManagementKeys(rec, profile); + rec.setKsMeta(generateKsMeta(rec)); + processKernelOptions(rec.getProfile()); + processOptionalProperties(rec); + + try { + rec.save(); } - else { - rec.setIpv6Autoconfiguration(false); + catch (XmlRpcException e) { + if (e.getCause() != null && e.getCause().getMessage() != null && + e.getCause().getMessage().contains("IP address duplicated")) { + return new ValidatorError( + "frontend.actions.systems.virt.duplicateipaddressvalue", + serverName); + } + throw e; } + /* + * This is a band-aid for the problem revealed in bug 846221. However + * the real fix involves creating a new System for the virtual guest + * instead of re-using the host System object, and I am unsure of what + * effects that would have. The System object is used when creating + * reActivation keys and setting up the cobbler SystemRecord network + * info among other things. No bugs have been reported in those areas + * yet, so I don't want to change something that has the potential to + * break a lot of things. + */ + if (saveCobblerId && server != null) { + server.setCobblerId(rec.getId()); + } + return new CobblerSyncCommand(user).store(); + } + + private void processRedHatManagementKeys(SystemRecord rec, Profile profile) { if (server != null) { if (this.activationKeys == null || this.activationKeys.isEmpty()) { - log.error("This cobbler profile does not " + - "have a redhat_management_key set "); + LOG.error("This cobbler profile ({}) does not " + + "have a redhat_management_key set ", profile.getId()); } else { rec.setRedHatManagementKey(Optional.of(activationKeys)); } } - if (!StringUtils.isBlank(getKickstartHost())) { - rec.setServer(Optional.of(getKickstartHost())); + } + + private SystemRecord getCobblerSystem(Profile cobblerProfile) { + SystemRecord rec = null; + // First lookup by MAC addr + if (server != null) { + rec = lookupExisting(server); + if (rec == null) { + // Next try by name + rec = SystemRecord.lookupByName(getCobblerConnection(user), + getCobblerSystemRecordName()); + } } - else { - rec.setServer(Optional.empty()); + + // Else, lets make a new system + if (rec == null) { + rec = SystemRecord.create(getCobblerConnection(), + getCobblerSystemRecordName(), cobblerProfile); } + return rec; + } + private Optional> generateKsMeta(SystemRecord rec) { // Setup the kickstart metadata so the URLs and activation key are setup - Map ksmeta; - if (rec.getKsMeta().isEmpty()) { - ksmeta = new HashMap<>(); - } - else { - ksmeta = rec.getKsMeta().get(); - } + Map ksmeta = rec.getKsMeta().orElseGet(HashMap::new); if (!StringUtils.isBlank(mediaPath)) { ksmeta.put(KickstartUrlHelper.COBBLER_MEDIA_VARIABLE, @@ -324,39 +377,42 @@ public ValidatorError store(boolean saveCobblerId) { if (this.ksDistro != null) { ksmeta.put(KickstartFormatter.KS_DISTRO, this.ksDistro); } - rec.setKsMeta(Optional.of(ksmeta)); - Profile recProfile = rec.getProfile(); - if (recProfile != null && "suse".equals(recProfile.getDistro().getBreed())) { - if (kernelOptions != null && kickstartHost != null && mediaPath != null) { - if (!kernelOptions.contains("install=")) { - kernelOptions = kernelOptions + " install=http://" + kickstartHost + - mediaPath; - } + return Optional.of(ksmeta); + } - Map resKopts = recProfile.getResolvedKernelOptions(); - boolean selfUpdateDisabled = false; - if (resKopts.getOrDefault("self_update", "Enabled").equals("0")) { - selfUpdateDisabled = true; - } - if (!(selfUpdateDisabled || kernelOptions.contains("self_update=") || ksData == null)) { - Optional installerUpdated = ksData.getTree().getChannel() - .getAccessibleChildrenFor(user) - .stream() - .filter(Channel::isInstallerUpdates) - .findFirst(); - if (installerUpdated.isPresent()) { - kernelOptions = kernelOptions + " self_update=http://" + kickstartHost + "/ks/dist/child/" + - installerUpdated.get().getLabel() + "/" + ksData.getTree().getLabel(); - } - else { - kernelOptions = kernelOptions + " self_update=0"; - } - } + private void processKernelOptions(Profile recProfile) { + if (recProfile != null && "suse".equals(recProfile.getDistro().getBreed()) && kernelOptions != null && + kickstartHost != null && mediaPath != null) { + if (!kernelOptions.contains("install=")) { + kernelOptions = String.format("%s install=http://%s%s", kernelOptions, kickstartHost, mediaPath); + } + + Map resKopts = recProfile.getResolvedKernelOptions(); + boolean selfUpdateDisabled = resKopts.getOrDefault("self_update", "Enabled").equals("0"); + if (!(selfUpdateDisabled || kernelOptions.contains("self_update=") || ksData == null)) { + Optional installerUpdated = ksData.getTree().getChannel() + .getAccessibleChildrenFor(user) + .stream() + .filter(Channel::isInstallerUpdates) + .findFirst(); + kernelOptions = installerUpdated.map( + channel -> String.format( + "%s self_update=http://%s/ks/dist/child/%s/%s", + kernelOptions, + kickstartHost, + channel.getLabel(), + ksData.getTree().getLabel() + )).orElseGet(() -> kernelOptions + " self_update=0"); } } + } + private void processOptionalProperties(SystemRecord rec) { + if (!StringUtils.isBlank(getKickstartHost())) { + rec.setServer(Optional.of(getKickstartHost())); + } if (server != null && server.getHostname() != null) { - rec.setHostName(getServer().getHostname()); + rec.setHostName(server.getHostname()); } else if (serverName != null) { rec.setHostName(serverName); @@ -367,41 +423,14 @@ else if (serverName != null) { if (postKernelOptions != null) { rec.setKernelOptionsPost(Optional.of(postKernelOptions)); } - // The comment is optional if (comment != null) { rec.setComment(comment); } - try { - rec.save(); - } - catch (XmlRpcException e) { - if (e.getCause() != null && e.getCause().getMessage() != null && - e.getCause().getMessage().contains("IP address duplicated")) { - return new ValidatorError( - "frontend.actions.systems.virt.duplicateipaddressvalue", - serverName); - } - throw e; - } - - /* - * This is a band-aid for the problem revealed in bug 846221. However - * the real fix involves creating a new System for the virtual guest - * instead of re-using the host System object, and I am unsure of what - * effects that would have. The System object is used when creating - * reActivation keys and setting up the cobbler SystemRecord network - * info among other things. No bugs have been reported in those areas - * yet, so I don't want to change something that has the potential to - * break a lot of things. - */ - if (saveCobblerId && server != null) { - server.setCobblerId(rec.getId()); - } - return new CobblerSyncCommand(user).store(); } /** * Get the cobbler system record name for a system + * * @return String name of cobbler system record */ public String getCobblerSystemRecordName() { @@ -410,89 +439,111 @@ public String getCobblerSystemRecordName() { /** * Get the cobbler system record name for a system + * * @param serverNameIn the name of the server - * @param orgIdIn the ID of the organization the server is in + * @param orgIdIn the ID of the organization the server is in * @return String name of cobbler system record */ public static String getCobblerSystemRecordName(String serverNameIn, Long orgIdIn) { String sep = ConfigDefaults.get().getCobblerNameSeparator(); String name = serverNameIn.replace(' ', '_'); - name = name.replace(' ', '_').replaceAll("[^a-zA-Z0-9_\\-\\.]", ""); + name = name.replace(' ', '_').replaceAll("[^a-zA-Z0-9_\\-.]", ""); return name + sep + orgIdIn; } protected void processNetworkInterfaces(SystemRecord rec, - Server serverIn) { + Server serverIn) { List nics = new LinkedList<>(); if (serverIn.getNetworkInterfaces() != null) { for (NetworkInterface n : serverIn.getNetworkInterfaces()) { - // don't create a physical network device for a bond - if (!n.isVirtBridge() && !n.isBond()) { - if (n.isPublic()) { - Network net = new Network(getCobblerConnection(), - n.getName()); - if (!n.getIPv4Addresses().isEmpty()) { - net.setIpAddress(n.getIPv4Addresses().get(0).getAddress()); - net.setNetmask(n.getIPv4Addresses().get(0).getNetmask()); - } - net.setMacAddress(n.getHwaddr()); - - if (!StringUtils.isBlank(networkInterface) && n.getName().equals(networkInterface)) { - net.setStaticNetwork(!isDhcp); - } - - ArrayList ipv6Addresses = n.getGlobalIpv6Addresses(); - if (!ipv6Addresses.isEmpty()) { - net.setIpv6Address(ipv6Addresses.get(0)); - ipv6Addresses.remove(0); - } - if (!ipv6Addresses.isEmpty()) { - net.setIpv6Secondaries(ipv6Addresses); - } - if (setupBridge && bridgeSlaves.contains(n.getName())) { - net.makeBondingSlave(); - net.setBondingMaster(bridgeName); - } - - nics.add(net); - } - else if (n.isMacValid() && n.getIPv4Addresses().isEmpty()) { - Network net = new Network(getCobblerConnection(), - n.getName()); - net.setMacAddress(n.getHwaddr()); - if (setupBridge && bridgeSlaves.contains(n.getName())) { - net.makeBondingSlave(); - net.setBondingMaster(bridgeName); - } - - nics.add(net); - } - } - else if (setupBridge && bridgeSlaves.contains(n.getName())) { - Network net = new Network(getCobblerConnection(), - n.getName()); - net.setMacAddress(n.getHwaddr()); - net.makeBondingSlave(); - net.setBondingMaster(bridgeName); - nics.add(net); - } + processSingleNetworkInterface(n).ifPresent(nics::add); } if (setupBridge) { - Network net = new Network(getCobblerConnection(), bridgeName); - net.makeBondingMaster(); - net.setBondingOptions(bridgeOptions); - net.setStaticNetwork(!isBridgeDhcp); - if (!isBridgeDhcp) { - net.setNetmask(bridgeNetmask); - net.setIpAddress(bridgeAddress); - rec.setGateway(bridgeGateway); - } - nics.add(net); + nics.add(setupBridgeInterface(rec)); } } rec.setNetworkInterfaces(nics); } + private Optional processSingleNetworkInterface(NetworkInterface networkInterfaceIn) { + // don't create a physical network device for a bond + if (!networkInterfaceIn.isVirtBridge() && !networkInterfaceIn.isBond()) { + if (networkInterfaceIn.isPublic()) { + return Optional.of(setupPublicInterface(networkInterfaceIn)); + } + else if (networkInterfaceIn.isMacValid() && networkInterfaceIn.getIPv4Addresses().isEmpty()) { + return Optional.of(setupInterface(networkInterfaceIn)); + } + } + else if (setupBridge && bridgeSlaves.contains(networkInterfaceIn.getName())) { + return Optional.of(setupBridgeSlaves(networkInterfaceIn)); + } + return Optional.empty(); + } + + private Network setupPublicInterface(NetworkInterface networkInterfaceIn) { + Network net = new Network(getCobblerConnection(), + networkInterfaceIn.getName()); + if (!networkInterfaceIn.getIPv4Addresses().isEmpty()) { + net.setIpAddress(networkInterfaceIn.getIPv4Addresses().get(0).getAddress()); + net.setNetmask(networkInterfaceIn.getIPv4Addresses().get(0).getNetmask()); + } + net.setMacAddress(networkInterfaceIn.getHwaddr()); + + if (!StringUtils.isBlank(networkInterface) && + networkInterfaceIn.getName().equals(networkInterface)) { + net.setStaticNetwork(!isDhcp); + } + + ArrayList ipv6Addresses = networkInterfaceIn.getGlobalIpv6Addresses(); + if (!ipv6Addresses.isEmpty()) { + net.setIpv6Address(ipv6Addresses.get(0)); + ipv6Addresses.remove(0); + } + if (!ipv6Addresses.isEmpty()) { + net.setIpv6Secondaries(ipv6Addresses); + } + if (setupBridge && bridgeSlaves.contains(networkInterfaceIn.getName())) { + net.makeBondingSlave(); + net.setBondingMaster(bridgeName); + } + return net; + } + + private Network setupInterface(NetworkInterface networkInterfaceIn) { + // Method name not optimal, however a precise name is not easy in this scenario. + Network net = new Network(getCobblerConnection(), + networkInterfaceIn.getName()); + net.setMacAddress(networkInterfaceIn.getHwaddr()); + if (setupBridge && bridgeSlaves.contains(networkInterfaceIn.getName())) { + net.makeBondingSlave(); + net.setBondingMaster(bridgeName); + } + return net; + } + + private Network setupBridgeSlaves(NetworkInterface networkInterfaceIn) { + Network net = new Network(getCobblerConnection(), + networkInterfaceIn.getName()); + net.setMacAddress(networkInterfaceIn.getHwaddr()); + net.makeBondingSlave(); + net.setBondingMaster(bridgeName); + return net; + } + + private Network setupBridgeInterface(SystemRecord systemRecordIn) { + Network net = new Network(getCobblerConnection(), bridgeName); + net.makeBondingMaster(); + net.setBondingOptions(bridgeOptions); + net.setStaticNetwork(!isBridgeDhcp); + if (!isBridgeDhcp) { + net.setNetmask(bridgeNetmask); + net.setIpAddress(bridgeAddress); + systemRecordIn.setGateway(bridgeGateway); + } + return net; + } + /** * @return the system */ @@ -532,6 +583,7 @@ public void setPostKernelOptions(String postKernelOptionsIn) { /** * Set the scheduled action associated to this command. + * * @param kickstartAction ks action associated to this command */ public void setScheduledAction(Action kickstartAction) { @@ -544,6 +596,7 @@ protected Action getScheduledAction() { /** * Setter for the comment. + * * @param commentIn the comment */ public void setComment(String commentIn) { @@ -552,6 +605,7 @@ public void setComment(String commentIn) { /** * Getter for the comment. + * * @return the comment */ public String getComment() { diff --git a/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/test/CobblerSystemCreateCommandTest.java b/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/test/CobblerSystemCreateCommandTest.java new file mode 100644 index 000000000000..a8776a34865f --- /dev/null +++ b/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/test/CobblerSystemCreateCommandTest.java @@ -0,0 +1,122 @@ +/* + * Copyright (c) 2023 SUSE LLC + * + * This software is licensed to you under the GNU General Public License, + * version 2 (GPLv2). There is NO WARRANTY for this software, express or + * implied, including the implied warranties of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2 + * along with this software; if not, see + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt. + * + * Red Hat trademarks are not licensed under GPLv2. No permission is + * granted to use or replicate Red Hat trademarks that are incorporated + * in this software or its documentation. + */ + +package com.redhat.rhn.manager.kickstart.cobbler.test; + +import com.redhat.rhn.common.validator.ValidatorError; +import com.redhat.rhn.domain.kickstart.KickstartData; +import com.redhat.rhn.domain.kickstart.test.KickstartDataTest; +import com.redhat.rhn.domain.server.Server; +import com.redhat.rhn.domain.server.test.ServerFactoryTest; +import com.redhat.rhn.domain.user.User; +import com.redhat.rhn.manager.kickstart.cobbler.CobblerSystemCreateCommand; +import com.redhat.rhn.manager.kickstart.cobbler.CobblerXMLRPCHelper; +import com.redhat.rhn.testing.BaseTestCaseWithUser; +import com.redhat.rhn.testing.UserTestUtils; + +import org.cobbler.CobblerConnection; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.LinkedList; + +public class CobblerSystemCreateCommandTest extends BaseTestCaseWithUser { + @Test + public void testConstructorDbPersistence() { + // Arrange + CobblerConnection connection = CobblerXMLRPCHelper.getConnection(user); + connection.invokeMethod("new_profile"); + var profileId = ((LinkedList) connection.invokeMethod("get_profiles")).get(0).get("uid"); + KickstartData k = KickstartDataTest.createTestKickstartData(user.getOrg()); + k.setCobblerId(profileId.toString()); + Server s = ServerFactoryTest.createTestServer(user, false); + + // Act + new CobblerSystemCreateCommand(user, k, s); + + // Assert + } + + @Test + public void testConstructorUnknown1() { + // Arrange + CobblerConnection connection = CobblerXMLRPCHelper.getConnection(user); + connection.invokeMethod("new_profile"); + var profileId = ((LinkedList) connection.invokeMethod("get_profiles")).get(0).get("uid"); + KickstartData k = KickstartDataTest.createTestKickstartData(user.getOrg()); + k.setCobblerId(profileId.toString()); + Server s = ServerFactoryTest.createTestServer(user, false); + + // Act + new CobblerSystemCreateCommand(user, s, k, "mediaPathIn", "activationKeysIn"); + + // Assert + } + + @Test + public void testConstructorReactivation() { + // Arrange + User admin = UserTestUtils.findNewUser("adminUser", "testOrg" + this.getClass().getSimpleName(), true); + KickstartData k = KickstartDataTest.createTestKickstartData(admin.getOrg()); + k.setCobblerId("test-id"); + Server s = ServerFactoryTest.createTestServer(admin, false); + + // Act + new CobblerSystemCreateCommand(user, s, "cobblerProfileName", k); + + // Assert + } + + @Test + public void testConstructorCobblerPassthrough() { + // Arrange + KickstartData k = KickstartDataTest.createTestKickstartData(user.getOrg()); + Server s = ServerFactoryTest.createTestServer(user, false); + + // Act + new CobblerSystemCreateCommand(user, "cobblerProfileName", k, s.getName(), 0L); + + // Assert + } + + @Test + public void testConstructorUnknown2() { + // Arrange + Server s = ServerFactoryTest.createTestServer(user, false); + + // Act + new CobblerSystemCreateCommand(user, s, "nameIn"); + + // Assert + } + + @Test + public void testStore() { + // Arrange + CobblerConnection connection = CobblerXMLRPCHelper.getConnection(user); + connection.invokeMethod("new_profile"); + String profileName = ((LinkedList) connection.invokeMethod("get_profiles")) + .get(0).get("name").toString(); + Server s = ServerFactoryTest.createTestServer(user, false); + CobblerSystemCreateCommand cobblerSystemCreateCommand = new CobblerSystemCreateCommand(user, s, profileName); + + // Act + ValidatorError error = cobblerSystemCreateCommand.store(); + + // Assert + Assertions.assertNull(error); + } +} diff --git a/java/code/src/org/cobbler/CobblerConnection.java b/java/code/src/org/cobbler/CobblerConnection.java index a39ecf3c839b..981441fb8ba4 100644 --- a/java/code/src/org/cobbler/CobblerConnection.java +++ b/java/code/src/org/cobbler/CobblerConnection.java @@ -142,11 +142,13 @@ public String login(String login, String password) { private Object invokeMethod(String procedureName, List args) { if (log.isDebugEnabled()) { List dbgArgs = new LinkedList<>(args); - String lastArg = (String) dbgArgs.get(dbgArgs.size() - 1); - if (lastArg.length() == 36 && lastArg.endsWith("==")) { - // probably a base64 token - dbgArgs.remove(dbgArgs.size() - 1); - dbgArgs.add(""); + if (!dbgArgs.isEmpty()) { + String lastArg = dbgArgs.get(dbgArgs.size() - 1).toString(); + if (lastArg.length() == 36 && lastArg.endsWith("==")) { + // probably a base64 token + dbgArgs.remove(dbgArgs.size() - 1); + dbgArgs.add(""); + } } log.debug("procedure: {} args: {}", procedureName, dbgArgs); } diff --git a/java/code/src/org/cobbler/CobblerObject.java b/java/code/src/org/cobbler/CobblerObject.java index 25323af2087f..45e043b263a7 100644 --- a/java/code/src/org/cobbler/CobblerObject.java +++ b/java/code/src/org/cobbler/CobblerObject.java @@ -171,7 +171,8 @@ protected static List> lookupDataMapsByCriteria( Map criteria = new HashMap<>(); criteria.put(critera, value); return (List>) - client.invokeTokenMethod(findMethod, criteria); + client.invokeTokenMethod(findMethod, criteria, true); + } diff --git a/java/code/src/org/cobbler/test/MockConnection.java b/java/code/src/org/cobbler/test/MockConnection.java index 1f44b20222d9..e287f39fff3f 100644 --- a/java/code/src/org/cobbler/test/MockConnection.java +++ b/java/code/src/org/cobbler/test/MockConnection.java @@ -366,6 +366,9 @@ private String newProfile() { String uid = random(); String xmlrpcHandle = random(); profile.put("uid", uid); + profile.put("name", random()); + String distro = newDistro(); + profile.put("distro", distro); log.debug("PROFILE: Created w/ uid {} returning handle {}", uid, xmlrpcHandle); @@ -426,6 +429,7 @@ private String newDistro() { distro.put("kernel_options_post", new HashMap<>()); distro.put("autoinstall_meta", new HashMap<>()); distro.put("redhat_management_key", ""); + distro.put("name", xmlrpcHandle); return xmlrpcHandle; } diff --git a/java/spacewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord b/java/spacewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord new file mode 100644 index 000000000000..c42a587faf40 --- /dev/null +++ b/java/spacewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord @@ -0,0 +1 @@ +- Fixes createSystemRecord XML-RPC API call so the Cobbler UID is persisted (bsc#1207532)