Note1: the "standard repositories" mean: Cluster, ClusterStorage, Workstation, VT.
Note2: This BZ describes two little bit distinct bugs, but related 1),2) and 3), so escalating them in one BZ.

Description of problem:

* 1) If we customize 'Download Location' during kickstart creation in 'Step 2: Distribution File Location', the '--baseurl' argument of 'repo' kickstart option doesn't reflect this change and gets hostname of server the Satellite is running on instead of our defined hostname.

* 2) If we customize '--url' argument of 'url' kickstart option in [some kickstart profile] -> "Kickstart Details" -> "Advanced Options", the '--baseurl' argument of 'repo' kickstart option doesn't get changed accordingly and gets hostname of server the Satellite is running on.

* 3) After every change to standard repositories in [some kickstart profile] -> "Kickstart Details" -> "Operating System" -> "Repositories", the 'url' kickstart option gets changed no matter that '--url' argument was previously set to different value (either through "Kickstart Details" -> "Advanced Options" or during kickstart creation described in 1)) so that the customized value (hostname) disappears and is finally replaced by hostname of server the Satellite is running on.


Version-Release number of selected component (if applicable):

Satellite 5.3 - spacewalk-java-0.5.44-76.el5sat


How reproducible:

always


Steps to Reproduce:

1) 
a. click on "Systems" -> "Kickstart" -> "create new kickstart profile", fill in requested data and click next
b. change "Default Download Location" to "Custom Download Location" and fill there e.g. http://foo.bar/ks/dist/ks-rhel-i386-server-5, click next, click next

2)
a. create kickstart profile with predefined settings (don't change anything)
b. click on "Systems" -> "Kickstart" -> "Profiles" -> [select kickstart profile] -> "Kickstart Details" -> "Advanced Options" and change url option from:
--url /ks/dist/ks-rhel-i386-server-5
to:
--url http://foo.bar/ks/dist/ks-rhel-i386-server-5
and click "Update Kickstart"

3)
a. use the profile created for point 2) - click on "Systems" -> "Kickstart" -> "Profiles" -> [select profile] -> "Kickstart Details" -> "Operating System"
b. make some changes to standard repositories - /uncheck some/all/ or /check some/all/ - simply made some change to them and click "Update Kickstart"


Actual results:

1) If you click on [kickstart profile generated for 1)] -> "Kickstart File", you can see that kickstart file contains:

=== <snip> ===
url --url http://foo.bar/ks/dist/ks-rhel-i386-server-5
...
repo --name=VT --baseurl=http://<real_hostname_of_Satellite_server>/ks/dist/ks-rhel-i386-server-5/VT
=== </snip> ===

2) If you click on [kickstart profile generated for 2)] -> "Kickstart File", you can see that kickstart file contains the same as for 1)

3) If you click on [kickstart profile generated for 2) after changes made for 3)] -> "Kickstart File", you can see that kickstart file contains (note --url argument gets changed from foo.bar to real hostname of Satellite server):

=== <snip> ===
url --url http://<real_hostname_of_Satellite_server>/ks/dist/ks-rhel-i386-server-5
...
repo --name=Cluster --baseurl=http://<real_hostname_of_Satellite_server>/ks/dist/ks-rhel-i386-server-5/Cluster
repo --name=ClusterStorage --baseurl=http://<real_hostname_of_Satellite_server>/ks/dist/ks-rhel-i386-server-5/ClusterStorage
repo --name=VT --baseurl=http://<real_hostname_of_Satellite_server>/ks/dist/ks-rhel-i386-server-5/VT
repo --name=Workstation --baseurl=http://<real_hostname_of_Satellite_server>/ks/dist/ks-rhel-i386-server-5/Workstation
=== </snip> ===


Expected results:

Hostname part of '--baseurl' argument should be the same as for '--url' argument automatically in case of standard repositories (not repositories, that could be /added/defined/ through form "Kickstart Details" -> "Advanced Options" -> "Custom options"):

1), 2)
=== <snip> ===
url --url http://foo.bar/ks/dist/ks-rhel-i386-server-5
...
repo --name=VT --baseurl=http://foo.bar/ks/dist/ks-rhel-i386-server-5/VT
=== </snip> ===

'--url' argument shouldn't forget its customuzed value:

3)
=== <snip> ===
url --url http://foo.bar/ks/dist/ks-rhel-i386-server-5
...
repo --name=Cluster --baseurl=http://foo.bar/ks/dist/ks-rhel-i386-server-5/Cluster
repo --name=ClusterStorage --baseurl=http://foo.bar/ks/dist/ks-rhel-i386-server-5/ClusterStorage
repo --name=VT --baseurl=http://foo.bar/ks/dist/ks-rhel-i386-server-5/VT
repo --name=Workstation --baseurl=http://foo.bar/ks/dist/ks-rhel-i386-server-5/Workstation
=== </snip> ===


Additional info:

<------------------------------------------------------------>
Temporary solution to get customized hostname from 'url --url=' also to 'repo --baseurl=' could be to un-check all repositories in

[some kickstart profile]-> "Kickstart Details"-> "Operating System" -> "Repositories"

and add them manually with corrected --baseurl argument to "Custom options" form:

[some kickstart profile]-> "Kickstart Details"-> "Advanced Options": "Custom options" form

e.g.:

repo --name=VT --baseurl=http://<some_hostname>/ks/dist/ks-rhel-i386-server-5/VT
repo --name=Workstation --baseurl=http://<some_hostname>/ks/dist/ks-rhel-i386-server-5/Workstation
</----------------------------------------------------------->


Investigation results:

The 1)st and 2)nd problem are related and caused by the fact that url and repo option are parsed and generated separately without any implemented relation (codepath evoked by clicking on "Systems" -> "Kickstart" -> "Profiles" -> [select kickstart profile] -> "Kickstart Details" -> "Advanced Options" -> "Update Kickstart" button) - note '<<<--' for my code comments:

spacewalk-java-0.5.44/code/webapp/WEB-INF/struts-config.xml:
=== <snip> ===
    <action path="/kickstart/KickstartOptionsEdit"
        scope="request"
        input="/WEB-INF/pages/kickstart/kickstartoptions.jsp"
        type="com.redhat.rhn.frontend.action.kickstart.KickstartAdvancedOptionsAction">
        <forward name="default"
                 path="/WEB-INF/pages/kickstart/kickstartoptions.jsp"/>
    </action>
=== </snip> ===

-> spacewalk-java-0.5.44/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartAdvancedOptionsAction.java:execute():
159                 cmd.store(); ->
spacewalk-java-0.5.44/code/src/com/redhat/rhn/manager/kickstart/BaseKickstartCommand.java:store():
92         KickstartFactory.saveKickstartData(ksData); ->
spacewalk-java-0.5.44/code/src/com/redhat/rhn/domain/kickstart/KickstartFactory.java:saveKickstartData():
 402         saveKickstartData(ksdataIn, null); ->
spacewalk-java-0.5.44/code/src/com/redhat/rhn/domain/kickstart/KickstartFactory.java:saveKickstartData():
 366             fileData = formatter.getFileData(); ->
spacewalk-java-0.5.44/code/src/com/redhat/rhn/manager/kickstart/KickstartFormatter.java:saveKickstartData():
207         buf.append(getCommands());

272     private String getCommands() {
...<snip>...
288             else if (cname.matches("url")) {
289                 String argVal = adjustUrlHost(command);  <<<--- see adjustUrlHost() code below
290                 commands.append(cname + SPACE + argVal + NEWLINE);
291             }
292             else if (cname.matches("repo")) {
293                 RepoInfo repo = RepoInfo.parse(command);
294                 commands.append(repo.getFormattedCommand(ksdata) + NEWLINE);  <<<--- see getFormattedCommand() code below
295             }

code for adjustUrlHost() (spacewalk-java-0.5.44/code/src/com/redhat/rhn/manager/kickstart/KickstartFormatter.java):
339     /**
340      * Adjust the URL hostname if necessary. Hostnames are stored in the db as relative
341      * paths if the user selects to use the default URL. When rendered we need to swap
342      * in the most appropriate hostname.
343      * 
344      * If this hostname appears to be customized, no change is made and we return the URL
345      * as is.
346      */
347     private String adjustUrlHost(KickstartCommand command) {
348         String argVal = command.getArguments();
349 
350         String urlLocation;
351         if (argVal.startsWith("--url")) {
352           urlLocation = argVal.substring("--url ".length());
353         }
354         else {
355           urlLocation = argVal;
356         }
357 
358         KickstartUrlHelper urlHelper = new KickstartUrlHelper(this.ksdata);
359 
360         log.debug("Got URL : " + command.getArguments());
361         log.debug("isRhnTree: " + this.ksdata.getTree().isRhnTree());
362         log.debug("Actual URL: " + urlLocation);
363 
364         if (urlLocation.startsWith("/")) {
365             log.debug("URL is not customized.");
366             log.debug("Formatting for view use.");
367             // /kickstart/dist/ks-rhel-i386-as-4-u2
368             StringBuffer url = new StringBuffer();
369             url.append("--url ");
370             url.append(urlHelper.getCobblerMediaUrl());
371             log.debug("constructed: " + url);
372             argVal = url.toString();
373         }
374         else {
375             log.debug("Just return the arg value."); <<<--- note - if we customize url, it simply returns, withou any mechanism to inform repo.getFormattedCommand() to construct diferrent hostname for repo
376         }
377         log.debug("returning url: " + argVal);
378         return argVal;
379     }

code for getFormattedCommand() (spacewalk-java-0.5.44/code/src/com/redhat/rhn/domain/kickstart/RepoInfo.java):
 97     /**
 98      * Returns the actual command line that will be used in the
 99      * KickstartFormatter.
100      * @param data KickstartData needed to generate the media url
101      * @return the formatted command
102      */
103     public String getFormattedCommand(KickstartData data) {
104         KickstartUrlHelper helper = new KickstartUrlHelper(data); <<<--- see KickstartUrlHelper() constructor code below (sets protocol and hostname)
105         return String.format("repo --name=%s --baseurl=%s", name,
106                                     helper.getRepoUrl(this)); <<<--- see getRepoUrl() code below
107     }

code for getRepoUrl() (spacewalk-java-0.5.44/code/src/com/redhat/rhn/manager/kickstart/KickstartUrlHelper.java):
245     /**
246      * Return the repo URL to be used in the formatted
247      * @param repo the repo object
248      * @return the repo url.
249      */
250     public String getRepoUrl(RepoInfo repo) {
251         return getCobblerMediaUrl() + "/" + repo.getUrl(); <<<--- see getCobblerMediaUrl() code below
252     }

code for getCobblerMediaUrl() (spacewalk-java-0.5.44/code/src/com/redhat/rhn/manager/kickstart/KickstartUrlHelper.java)
229     /**
230      * Get the cobbler style --url:
231      * 
232      * http://$http_server/$media_url
233      * 
234      * To be filled out by cobbler.  not spacewalk.
235      * 
236      * @return String url , cobbler style: http://$http_server/$media_url
237      */
238     public String getCobblerMediaUrl() {
239         StringBuilder url = new StringBuilder();
240         url.append(protocol + host + "$" + COBBLER_MEDIA_VARIABLE); <<<--- see constructors below to know how host is being constructed
241         log.debug("returning: " + url);
242         return url.toString();
243     }

code for KickstartUrlHelper() constructor:
 59     /**
 60      * Constructor.
 61      *
 62      * @param ksDataIn who's URL you desire.
 63      */
 64     public KickstartUrlHelper(KickstartData ksDataIn) {
 65         this(ksDataIn, COBBLER_SERVER_VARIABLE); <<<--- COBBLER_SERVER_VARIABLE = $http_server
 66     }
...snip...
 78     /**
 79      * Constructor.
 80      * 
 81      * @param ksDataIn who's URL you desire.
 82      * @param hostIn who is hosting the kickstart file.
 83      */
 84     public KickstartUrlHelper(KickstartData ksDataIn, String hostIn) {
 85         this.ksData = ksDataIn;
 86         this.host = hostIn;
 87         if (this.ksData != null) {
 88             this.ksTree = ksDataIn.getTree();
 89         }
 90         this.protocol = "http://";
 91     }

From above code we can see that both 'url' and 'repo' share hostname of Sattelite server by default through COBBLER_SERVER_VARIABLE, but not hostname customized through '--url' argument of 'url' kickstart option.

The 3)rd problem is caused by the fact that customized '--url' argument of 'url' kickstart option is finally saved nowhere (beside the generated kickstart file) and everytime the file gets regenerated it automatically lost any previous customized value, because the file doesn't gets regenerated from existing file, but using values from database/cobbler, which doesn't contain customized hostname, but only relative path to repository in a form /ks/dist/ks-rhel-* and Satellite hostname provided by cobbler.