From ae21a7a21df84f0c7c262e053d9f10a3804c0225 Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Thu, 2 Mar 2023 09:12:59 +0100 Subject: [PATCH 01/13] fix: API - createSystemRecord This commit addresses a regression that was introduced in 92e5a1996cf4968ef3660b53a2f01ed52e605d4c. When the server object is not passed to CobblerSystemCreateCommand the Cobbler UID is not persisted in the database which leads to Cobbler rejecting the API requests that SUSE Manager does. This doesn't happen via the UI since it passes the server object via a different constructor than the XML-RPC API. --- .../frontend/xmlrpc/system/SystemHandler.java | 27 ++- .../cobbler/CobblerSystemCreateCommand.java | 166 ++++++++++-------- 2 files changed, 116 insertions(+), 77 deletions(-) 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/manager/kickstart/cobbler/CobblerSystemCreateCommand.java b/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/CobblerSystemCreateCommand.java index 0aed19a55f44..b92f166b463c 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 @@ -55,8 +55,8 @@ public class CobblerSystemCreateCommand extends CobblerCommand { private static 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 +79,72 @@ 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 TODO + * @param cobblerProfileNameIn TODO */ - 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 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 */ 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,30 +152,26 @@ 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 = ""; + StringBuilder keys = new StringBuilder(); 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(); + keys = new StringBuilder(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(); + keys.append(",").append(keyTmp.getKey()); } } } - this.activationKeys = keys; + this.activationKeys = keys.toString(); } /** @@ -190,27 +185,23 @@ public CobblerSystemCreateCommand(User userIn, Server serverIn, String cobblerPr * @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 = ""; + StringBuilder keys = new StringBuilder(); 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 += ","; + if (!keys.toString().isBlank()) { + keys.append(","); } - keys += keyTmp.getKey(); + keys.append(keyTmp.getKey()); } } } - this.activationKeys = keys; + this.activationKeys = keys.toString(); } @@ -218,15 +209,48 @@ public CobblerSystemCreateCommand(User userIn, String cobblerProfileName, Kickst * Constructor * @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); + } + + /** + * @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; } /** From e9edff14d4b255b649f596c4b0d43ecb570b07d2 Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Thu, 2 Mar 2023 10:03:46 +0100 Subject: [PATCH 02/13] Sonarlint --- .../cobbler/CobblerSystemCreateCommand.java | 450 ++++++++++-------- 1 file changed, 240 insertions(+), 210 deletions(-) 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 b92f166b463c..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,12 +47,11 @@ 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 final String serverName; @@ -81,11 +80,11 @@ public class CobblerSystemCreateCommand extends CobblerCommand { /** * 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 TODO - * @param cobblerProfileNameIn TODO + * @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. */ private CobblerSystemCreateCommand( User userIn, @@ -104,7 +103,7 @@ private CobblerSystemCreateCommand( /** * Constructor that persists the Cobbler ID in the database after creating it via {@link #store()}. * - * @param userIn The user that creates the system. + * @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. */ @@ -121,12 +120,13 @@ public CobblerSystemCreateCommand(User userIn, KickstartData ksDataIn, Server se /** * Constructor - * @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 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) { @@ -154,62 +154,41 @@ public CobblerSystemCreateCommand(User userIn, Server serverIn, String cobblerPr KickstartData ksDataIn) { this(userIn, serverIn, serverIn.getName(), serverIn.getOrgId(), cobblerProfileName); this.mediaPath = null; - StringBuilder keys = new StringBuilder(); String note = "Reactivation key for " + serverName + "."; ActivationKey key = ActivationKeyManager.getInstance(). createNewReActivationKey(UserFactory.findRandomOrgAdmin( serverIn.getOrg()), serverIn, note); key.setUsageLimit(1L); - log.debug("created reactivation key: {}", key.getKey()); - keys = new StringBuilder(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.append(",").append(keyTmp.getKey()); - } - } - } - this.activationKeys = keys.toString(); + 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) { this(userIn, null, serverNameIn, orgIdIn, cobblerProfileName); this.mediaPath = null; - StringBuilder keys = new StringBuilder(); this.ksData = ksDataIn; - if (this.ksData != null) { - for (Token token : this.ksData.getDefaultRegTokens()) { - ActivationKey keyTmp = ActivationKeyFactory.lookupByToken(token); - if (keyTmp != null) { - if (!keys.toString().isBlank()) { - keys.append(","); - } - keys.append(keyTmp.getKey()); - } - } - } - this.activationKeys = keys.toString(); + 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 server. + * @param nameIn profile name to associate with server. */ public CobblerSystemCreateCommand(User userIn, Server serverIn, String nameIn) { @@ -217,13 +196,36 @@ public CobblerSystemCreateCommand(User userIn, Server serverIn, } /** - * @param dhcp true if the network type is dhcp + * 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 + * @param useIpv6GatewayIn whether to use ipv6 gateway + * @param ksDistroIn distro to be provisioned */ public void setNetworkInfo(boolean dhcp, String networkInterfaceIn, - boolean useIpv6GatewayIn, String ksDistroIn) { + boolean useIpv6GatewayIn, String ksDistroIn) { isDhcp = dhcp; networkInterface = networkInterfaceIn; useIpv6Gateway = useIpv6GatewayIn; @@ -231,18 +233,18 @@ public void setNetworkInfo(boolean dhcp, String networkInterfaceIn, } /** - * @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 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) + * @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) { + List slaves, String options, boolean isBridgeDhcpIn, + String address, String netmask, String gateway) { setupBridge = doBridge; bridgeName = name; bridgeSlaves = slaves; @@ -255,6 +257,7 @@ public void setBridgeInfo(boolean doBridge, String name, /** * Store the System to cobbler + * * @return ValidatorError if the store failed. */ @Override @@ -264,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); @@ -301,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, @@ -348,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); @@ -391,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() { @@ -434,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 */ @@ -556,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) { @@ -568,6 +596,7 @@ protected Action getScheduledAction() { /** * Setter for the comment. + * * @param commentIn the comment */ public void setComment(String commentIn) { @@ -576,6 +605,7 @@ public void setComment(String commentIn) { /** * Getter for the comment. + * * @return the comment */ public String getComment() { From 0038381b96059dca3737f09bbc8c0ecc5284e1e3 Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Wed, 31 May 2023 10:20:00 +0200 Subject: [PATCH 03/13] spacewalk-java: Add test dependency Mockito --- java/buildconf/ivy/ivy-suse.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 @@ - + From e7658d5f6fe7b980bb43a20f3e8a7c0b9018c9bd Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Fri, 3 Mar 2023 10:52:09 +0100 Subject: [PATCH 04/13] spacewalk-java: Add test skeletons for SystemHandler and CobblerSystemCreateCommand --- .../xmlrpc/system/test/SystemHandlerTest.java | 71 +++++++++++- .../test/CobblerSystemCreateCommandTest.java | 101 ++++++++++++++++++ 2 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 java/code/src/com/redhat/rhn/manager/kickstart/cobbler/test/CobblerSystemCreateCommandTest.java 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..f9eeea59c40e 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 @@ -25,6 +25,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mockConstruction; import com.redhat.rhn.FaultException; import com.redhat.rhn.common.client.ClientCertificate; @@ -178,6 +180,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; @@ -187,6 +190,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.MockedConstruction; import java.nio.file.Files; import java.nio.file.Path; @@ -3195,19 +3199,84 @@ public void testCreateSystemProfileNoHwAddress() throws Exception { } } + @Test + public void testCreateSystemRecordRegistered() throws Exception { + // Arrange + try (MockedConstruction clientMock = mockConstruction( + CobblerConnection.class, + (mock, context) -> { + HashMap criteria = new HashMap<>(); + criteria.put("uid", "my_uid"); + HashMap resultProfile = new HashMap<>(); + resultProfile.put("uid", "my_uid"); + resultProfile.put("name", "testprofile"); + given(mock.invokeMethod( + "find_profile", + criteria, + false, + "token" + )).willReturn(resultProfile); + })) { + SystemHandler mockedHandler = getMockedHandler(); + KickstartData k = KickstartDataTest.createTestKickstartData(admin.getOrg()); + k.setCobblerId("my_uid"); + 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 + SystemHandler mockedHandler = getMockedHandler(); + String systemName = "test system"; + mockedHandler.createSystemProfile( + admin, + systemName, + Collections.singletonMap("hwAddress", "aa:bb:cc:dd:ee:02") + ); + KickstartData k = KickstartDataTest.createTestKickstartData(admin.getOrg()); + + // Act + int result = mockedHandler.createSystemRecord( + admin, + systemName, + k.getLabel(), + "", + "", + new LinkedList<>() + ); + + // 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/test/CobblerSystemCreateCommandTest.java b/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/test/CobblerSystemCreateCommandTest.java new file mode 100644 index 000000000000..61ec57d210d8 --- /dev/null +++ b/java/code/src/com/redhat/rhn/manager/kickstart/cobbler/test/CobblerSystemCreateCommandTest.java @@ -0,0 +1,101 @@ +/* + * 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.manager.kickstart.cobbler.CobblerSystemCreateCommand; +import com.redhat.rhn.testing.BaseTestCaseWithUser; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class CobblerSystemCreateCommandTest extends BaseTestCaseWithUser { + @Test + public void testConstructorDbPersistence() { + // Arrange + KickstartData k = KickstartDataTest.createTestKickstartData(user.getOrg()); + Server s = ServerFactoryTest.createTestServer(user, false); + + // Act + new CobblerSystemCreateCommand(user, k, s); + + // Assert + } + + @Test + public void testConstructorUnknown1() { + // Arrange + KickstartData k = KickstartDataTest.createTestKickstartData(user.getOrg()); + Server s = ServerFactoryTest.createTestServer(user, false); + + // Act + new CobblerSystemCreateCommand(user, s, k, "mediaPathIn", "activationKeysIn"); + + // Assert + } + + @Test + public void testConstructorReactivation() { + // Arrange + KickstartData k = KickstartDataTest.createTestKickstartData(user.getOrg()); + Server s = ServerFactoryTest.createTestServer(user, 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 + Server s = ServerFactoryTest.createTestServer(user, false); + CobblerSystemCreateCommand cobblerSystemCreateCommand = new CobblerSystemCreateCommand(user, s, "nameIn"); + + // Act + ValidatorError error = cobblerSystemCreateCommand.store(); + + // Assert + Assertions.assertNull(error); + } +} From 762553490438bc7a78d8aa4b0519c929a9fdaddd Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Wed, 26 Apr 2023 11:58:15 +0200 Subject: [PATCH 05/13] spacewalk-java: Fixup find_* method parameters --- java/code/src/org/cobbler/CobblerObject.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/code/src/org/cobbler/CobblerObject.java b/java/code/src/org/cobbler/CobblerObject.java index 25323af2087f..f625a1e48d5c 100644 --- a/java/code/src/org/cobbler/CobblerObject.java +++ b/java/code/src/org/cobbler/CobblerObject.java @@ -171,7 +171,7 @@ protected static List> lookupDataMapsByCriteria( Map criteria = new HashMap<>(); criteria.put(critera, value); return (List>) - client.invokeTokenMethod(findMethod, criteria); + client.invokeTokenMethod(findMethod, criteria, false); } From 6124332ae2b21bcdcaecae6568e703e128a4b8d7 Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Wed, 24 May 2023 15:01:45 +0200 Subject: [PATCH 06/13] spacewalk-java: Document when User can be null for getCreator() --- java/code/src/com/redhat/rhn/domain/server/Server.java | 4 ++++ 1 file changed, 4 insertions(+) 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 fb223554c717..71892a908242 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() { From 2066ac9186e775fb45b15e97e59c35f4ef0a9746 Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Thu, 25 May 2023 10:20:37 +0200 Subject: [PATCH 07/13] spacewalk-java: Add @Nullable Annotations --- .../src/com/redhat/rhn/domain/kickstart/KickstartData.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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..5a0aac9bc2a9 100644 --- a/java/code/src/com/redhat/rhn/domain/kickstart/KickstartData.java +++ b/java/code/src/com/redhat/rhn/domain/kickstart/KickstartData.java @@ -38,6 +38,7 @@ import org.cobbler.CobblerConnection; import org.cobbler.Profile; +import javax.annotation.Nullable; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -1458,7 +1459,8 @@ public String getDefaultVirtBridge() { * taskomatic. * @return the Profile associated to this ks data */ - public Profile getCobblerObject(User user) { + @Nullable + public Profile getCobblerObject(@Nullable User user) { if (StringUtils.isBlank(getCobblerId())) { return null; } From 3cc3550a18be2a60255d886444587667dafdfc49 Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Wed, 31 May 2023 10:19:28 +0200 Subject: [PATCH 08/13] spacewalk-java: Enable test makefile to work with podman --- java/Makefile.docker | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 From 98dc118a24df66c852627cbb54febb4b5ad693e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yeray=20Guti=C3=A9rrez=20Cedr=C3=A9s?= Date: Thu, 5 Oct 2023 10:33:05 +0100 Subject: [PATCH 09/13] Apply new changelog format --- java/buildconf/ivy/ivy-suse.xml | 5 +++++ .../src/com/redhat/rhn/domain/kickstart/KickstartData.java | 5 ++--- ...ewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 java/spacewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord diff --git a/java/buildconf/ivy/ivy-suse.xml b/java/buildconf/ivy/ivy-suse.xml index 05c1cdd2f442..8f5aadfb8afd 100644 --- a/java/buildconf/ivy/ivy-suse.xml +++ b/java/buildconf/ivy/ivy-suse.xml @@ -148,5 +148,10 @@ + + + + + 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 5a0aac9bc2a9..db3367c7862c 100644 --- a/java/code/src/com/redhat/rhn/domain/kickstart/KickstartData.java +++ b/java/code/src/com/redhat/rhn/domain/kickstart/KickstartData.java @@ -38,7 +38,6 @@ import org.cobbler.CobblerConnection; import org.cobbler.Profile; -import javax.annotation.Nullable; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -1459,8 +1458,8 @@ public String getDefaultVirtBridge() { * taskomatic. * @return the Profile associated to this ks data */ - @Nullable - public Profile getCobblerObject(@Nullable User user) { + + public Profile getCobblerObject(User user) { if (StringUtils.isBlank(getCobblerId())) { return null; } 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..163fe8ea578a --- /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 From 39b468cf865f800dee23fe1ae651213e9531dbd4 Mon Sep 17 00:00:00 2001 From: Welder Luz Date: Mon, 23 Oct 2023 19:31:07 -0300 Subject: [PATCH 10/13] Remove mockito and fix unit tests --- java/buildconf/ivy/ivy-suse.xml | 5 -- .../xmlrpc/system/test/SystemHandlerTest.java | 68 ++++++++----------- .../test/CobblerSystemCreateCommandTest.java | 27 +++++++- .../src/org/cobbler/test/MockConnection.java | 4 ++ 4 files changed, 57 insertions(+), 47 deletions(-) diff --git a/java/buildconf/ivy/ivy-suse.xml b/java/buildconf/ivy/ivy-suse.xml index 8f5aadfb8afd..05c1cdd2f442 100644 --- a/java/buildconf/ivy/ivy-suse.xml +++ b/java/buildconf/ivy/ivy-suse.xml @@ -148,10 +148,5 @@ - - - - - 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 f9eeea59c40e..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 @@ -25,8 +25,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mockConstruction; import com.redhat.rhn.FaultException; import com.redhat.rhn.common.client.ClientCertificate; @@ -148,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; @@ -190,7 +189,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; -import org.mockito.MockedConstruction; import java.nio.file.Files; import java.nio.file.Path; @@ -3202,50 +3200,42 @@ public void testCreateSystemProfileNoHwAddress() throws Exception { @Test public void testCreateSystemRecordRegistered() throws Exception { // Arrange - try (MockedConstruction clientMock = mockConstruction( - CobblerConnection.class, - (mock, context) -> { - HashMap criteria = new HashMap<>(); - criteria.put("uid", "my_uid"); - HashMap resultProfile = new HashMap<>(); - resultProfile.put("uid", "my_uid"); - resultProfile.put("name", "testprofile"); - given(mock.invokeMethod( - "find_profile", - criteria, - false, - "token" - )).willReturn(resultProfile); - })) { - SystemHandler mockedHandler = getMockedHandler(); - KickstartData k = KickstartDataTest.createTestKickstartData(admin.getOrg()); - k.setCobblerId("my_uid"); - 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); - } + 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"; - mockedHandler.createSystemProfile( - admin, - systemName, - Collections.singletonMap("hwAddress", "aa:bb:cc:dd:ee:02") - ); 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, @@ -3253,7 +3243,7 @@ public void testCreateSystemRecordUnregistered() throws Exception { k.getLabel(), "", "", - new LinkedList<>() + List.of(netDevices) ); // Assert 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 index 61ec57d210d8..a8776a34865f 100644 --- 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 @@ -20,17 +20,28 @@ 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 @@ -42,7 +53,11 @@ public void testConstructorDbPersistence() { @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 @@ -54,8 +69,10 @@ public void testConstructorUnknown1() { @Test public void testConstructorReactivation() { // Arrange - KickstartData k = KickstartDataTest.createTestKickstartData(user.getOrg()); - Server s = ServerFactoryTest.createTestServer(user, false); + 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); @@ -89,8 +106,12 @@ public void testConstructorUnknown2() { @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, "nameIn"); + CobblerSystemCreateCommand cobblerSystemCreateCommand = new CobblerSystemCreateCommand(user, s, profileName); // Act ValidatorError error = cobblerSystemCreateCommand.store(); 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; } From 272913b750e3e15b24636bdda28a570fd009a6cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Thu, 26 Oct 2023 16:55:07 +0100 Subject: [PATCH 11/13] Add missing bsc reference to changelog --- ...pacewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/spacewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord b/java/spacewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord index 163fe8ea578a..c42a587faf40 100644 --- a/java/spacewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord +++ b/java/spacewalk-java.changes.ycedres.fix-xml-rpc-createSystemRecord @@ -1 +1 @@ -- Fixes createSystemRecord XML-RPC API call so the Cobbler UID is persisted +- Fixes createSystemRecord XML-RPC API call so the Cobbler UID is persisted (bsc#1207532) From 5480b0f7221900f604b3861fa9f29ab956c7aacc Mon Sep 17 00:00:00 2001 From: Michael Calmer Date: Sat, 4 Nov 2023 11:57:57 +0100 Subject: [PATCH 12/13] fix option handling in debug log mode --- java/code/src/org/cobbler/CobblerConnection.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) 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); } From 838abe3fd08351e19e104e742852cdcf1e398649 Mon Sep 17 00:00:00 2001 From: Michael Calmer Date: Sat, 4 Nov 2023 11:58:34 +0100 Subject: [PATCH 13/13] request expanded object when we expect a Map as return value --- java/code/src/org/cobbler/CobblerObject.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/java/code/src/org/cobbler/CobblerObject.java b/java/code/src/org/cobbler/CobblerObject.java index f625a1e48d5c..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, false); + client.invokeTokenMethod(findMethod, criteria, true); + }