From the cobbler command line, it is not possible to add/associate a
kickstart file that has capital letters. See the example below:
cobbler profile edit --name=My-Profile --kickstart=/var/lib/cobbler/kickstarts/My-Kickstart-File-With-Caps.ks
Could this be fixed in future versions?
Thanks, Razi
From the cobbler command line, it is not possible to add/associate a
kickstart file that has capital letters. See the example below:
cobbler profile edit --name=My-Profile --kickstart=/var/lib/cobbler/kickstarts/My-Kickstart-File-With-Caps.ks
Could this be fixed in future versions?
Thanks, Razi
On Wed, 21 Mar 2012 15:25:57 -0400, Razi Khaja wrote:
From the cobbler command line, it is not possible to add/associate a kickstart file that has capital letters. See the example below:
cobbler profile edit --name=My-Profile
--kickstart=/var/lib/cobbler/kickstarts/My-Kickstart-File-With-Caps.ks
Could this be fixed in future versions?
Well, it works perfectly well for me.
Do you receive an error message?
cobbler profile add --name=My-Profile --distro=ks-rhel-x86_64-server-5-57 --kickstart=/var/lib/cobbler/kickstarts/Kickstart-With-Capitals.ks
On Thu, Mar 29, 2012 at 4:52 AM, Stuart Sears stuart@sjsears.com wrote:
On Wed, 21 Mar 2012 15:25:57 -0400, Razi Khaja wrote:
From the cobbler command line, it is not possible to add/associate a kickstart file that has capital letters. See the example below:
cobbler profile edit --name=My-Profile
--kickstart=/var/lib/cobbler/kickstarts/My-Kickstart-File-With-Caps.ks
Could this be fixed in future versions?
Well, it works perfectly well for me.
Do you receive an error message?
cobbler profile add --name=My-Profile --distro=ks-rhel-x86_64-server-5-57 --kickstart=/var/lib/cobbler/kickstarts/Kickstart-With-Capitals.ks
I do:
# cobbler profile edit --name=test --kickstart=/var/lib/cobbler/kickstarts/Test-File.ks kickstart not found: None
I'm using the master branch.
On 29/03/12 11:57, Razi Khaja wrote:
I am using the master branch as well from github. Razi
then this has been introduced since the version I'm using (admittedly part of RHN satellite, so not really very current at all)
browsing the source on the home cobbler server (which is also upstream master), this is where the problem lies, as I see it:
########################## def find_kickstart(url): """ Check if a kickstart url looks like an http, ftp, nfs or local path. If a local path is used, cobbler will copy the kickstart and serve it over http.
Return None if the url format does not look valid. """ if url is None: return None x = url.lower().lstrip() for y in ["http://", "nfs://", "ftp://", "/"]: if x.startswith(y): if x.startswith("/") and not os.path.isfile(x): return None return x return None ###########################
We are automatically lower-casing kickstart names before searching, so of course this doesn't work, unless we accidentally happen to have a non-uppercased match available in the same directory (Which would be odd, tbh)
Anyone know why we do that? It seems curious to me.
Stuart
On Thu, Mar 29, 2012 at 8:00 AM, Stuart Sears stuart@sjsears.com wrote:
On 29/03/12 11:57, Razi Khaja wrote:
I am using the master branch as well from github. Razi
then this has been introduced since the version I'm using (admittedly part of RHN satellite, so not really very current at all)
browsing the source on the home cobbler server (which is also upstream master), this is where the problem lies, as I see it:
########################## def find_kickstart(url): """ Check if a kickstart url looks like an http, ftp, nfs or local path. If a local path is used, cobbler will copy the kickstart and serve it over http.
Return None if the url format does not look valid. """ if url is None: return None x = url.lower().lstrip() for y in ["http://", "nfs://", "ftp://", "/"]: if x.startswith(y): if x.startswith("/") and not os.path.isfile(x): return None return x return None ###########################
We are automatically lower-casing kickstart names before searching, so of course this doesn't work, unless we accidentally happen to have a non-uppercased match available in the same directory (Which would be odd, tbh)
Anyone know why we do that? It seems curious to me.
edf29250 cobbler/utils.py (James Cammarata 2012-03-04 13:45:06 -0600 435) x = url.lower().lstrip()
This was the commit: [BUGFIX] Fix for issue #42 - kickstart not found error when path has leading space. But I just added the lstrip(), and not the lower() (from commit c09652a11eec8a14d6dded1ee743806ef4c1c2b4):
- x = url.lower() + x = url.lower().lstrip()
036eb874 cobbler/util.py (Michael DeHaan 2006-05-05 15:41:27 -0400 435) x = url.lower()
# git log -1 036eb874 commit 036eb874ffbaf70236427ebe7c10fb883706e291 Author: Michael DeHaan mdehaan@redhat.com Date: Fri May 5 15:41:27 2006 -0400
Interim commit while refactoring. This breaks lots of stuff.
So the .lower() has been in there forever. The real issue is that I changed it to return x, whereas before it was returning the url variable, which did not work if the url started with whitespace. The lower() just seems to make sure you get http instead of HtTp or something random a user may enter, so I'll fix this to be a bit smarter. Sorry for the breakage.
On Thu, Mar 29, 2012 at 6:50 PM, James Cammarata jimi@sngx.net wrote:
So the .lower() has been in there forever. The real issue is that I changed it to return x, whereas before it was returning the url variable, which did not work if the url started with whitespace. The lower() just seems to make sure you get http instead of HtTp or something random a user may enter, so I'll fix this to be a bit smarter. Sorry for the breakage.
Here's my fix:
# git diff diff --git a/cobbler/utils.py b/cobbler/utils.py index f30cdff..7a83317 100644 --- a/cobbler/utils.py +++ b/cobbler/utils.py @@ -432,8 +432,11 @@ def find_kickstart(url): """ if url is None: return None - x = url.lower().lstrip() + x = url.lstrip() for y in ["http://", "nfs://", "ftp://", "/"]: + # make sure we get a lower-case protocol without + # affecting the rest of the string + x = re.sub(r"(?i)%s" % y, y, x, count=1) if x.startswith(y): if x.startswith("/") and not os.path.isfile(x): return None
Test results:
$ cobbler profile edit --name=test --kickstart=/var/lib/cobbler/kickstarts/Test-File.ks $ cobbler profile report --name=test | grep ^Kickstart Kickstart : /var/lib/cobbler/kickstarts/Test-File.ks Kickstart Metadata : {}
This has been merged into the master branch.
On 30/03/12 06:14, James Cammarata wrote:
On Thu, Mar 29, 2012 at 6:50 PM, James Cammarata jimi@sngx.net wrote:
So the .lower() has been in there forever. The real issue is that I changed it to return x, whereas before it was returning the url variable, which did not work if the url started with whitespace. The lower() just seems to make sure you get http instead of HtTp or something random a user may enter, so I'll fix this to be a bit smarter. Sorry for the breakage.
Here's my fix:
# git diff diff --git a/cobbler/utils.py b/cobbler/utils.py index f30cdff..7a83317 100644 --- a/cobbler/utils.py +++ b/cobbler/utils.py @@ -432,8 +432,11 @@ def find_kickstart(url): """ if url is None: return None
- x = url.lower().lstrip()
- x = url.lstrip() for y in ["http://", "nfs://", "ftp://", "/"]:
# make sure we get a lower-case protocol without
# affecting the rest of the string
x = re.sub(r"(?i)%s" % y, y, x, count=1) if x.startswith(y): if x.startswith("/") and not os.path.isfile(x): return None
Test results:
$ cobbler profile edit --name=test --kickstart=/var/lib/cobbler/kickstarts/Test-File.ks $ cobbler profile report --name=test | grep ^Kickstart Kickstart : /var/lib/cobbler/kickstarts/Test-File.ks Kickstart Metadata : {}
really?
my take on it was more like this: $ git diff diff --git a/cobbler/utils.py b/cobbler/utils.py index f30cdff..d7f1da1 100644 --- a/cobbler/utils.py +++ b/cobbler/utils.py @@ -432,12 +432,13 @@ def find_kickstart(url): """ if url is None: return None - x = url.lower().lstrip() - for y in ["http://", "nfs://", "ftp://", "/"]: - if x.startswith(y): - if x.startswith("/") and not os.path.isfile(x): - return None - return x + proto, path = urllib2.splittype(url.lstrip()) + if proto: + if proto in ["http", "nfs", "ftp"]: + return '%s:%s' %( proto.lower(), path) + else: + if path.startswith('/') and os.path.isfile(path): + return path return None
with the same test results.
still, as long as it works now...
Stuart
cobbler@lists.fedorahosted.org