Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass this through to directories. #539

Merged
merged 1 commit into from
Jan 23, 2014
Merged

Pass this through to directories. #539

merged 1 commit into from
Jan 23, 2014

Conversation

apenney
Copy link
Contributor

@apenney apenney commented Dec 17, 2013

The current code allows you to set index_options but then ignores it.
Based on the git log for the commit that added this option it was
intended to be passed into directories. The test that caught this
will be merged into spec/acceptance/vhost_spec.rb in a seperate PR.

You can reproduce the original failure with:

class { 'apache': }
host { 'test.server': ip => '127.0.0.1' }
apache::vhost { 'test.server':
  docroot    => '/tmp',
  index_options    => ['Charset=UTF-8'],
}

@igalic
Copy link
Contributor

igalic commented Dec 17, 2013

When index_options is passed in apache::vhost, it should end up in
When index_options is passed as part of apache::vhost's directories, it should end up in that VirtualHost's

@apenney
Copy link
Contributor Author

apenney commented Dec 17, 2013

@anyone-can-test Did you want this option to be in the main vhost or inside directories exclusively?

@hunner
Copy link
Contributor

hunner commented Dec 17, 2013

Really, the directories parameter is the best way to allow arbitrary directory settings. 99% of directory-specific values that are passed at top scope should probably be more or less removed as they are too restrictive or specific for the general case.

The current code allows you to set index_options but then ignores it.
Based on the git log for the commit that added this option it was
intended to be passed into directories.  The test that caught this
will be merged into spec/acceptance/vhost_spec.rb in a seperate PR.

You can reproduce the original failure with:

class { 'apache': }
host { 'test.server': ip => '127.0.0.1' }
apache::vhost { 'test.server':
  docroot    => '/tmp',
  index_options    => ['Charset=UTF-8'],
}

We're just going to remove them because they can be passed directly
into directory instead of indirectly.
@apenney
Copy link
Contributor Author

apenney commented Dec 17, 2013

@hunner Yeah, sold.

@anyone-can-test
Copy link
Contributor

I'm not sure if index_options can be used under apache:vhost.
My intention is that index_options is included in directories.

@anyone-can-test
Copy link
Contributor

if index_options, index_order_default is removed, how can I add IndexOptions, IndexOrderDefault to Directory setting?

@igalic
Copy link
Contributor

igalic commented Dec 18, 2013

To reiterate, IndexOptions can be set in server, vhost, and directory context (also .htaccess, but we don't care).

This means: Someone may want to set it on the entire vhost, and then tweak or disallow it on one specific directory. At the risk of sounding like a broken record, I think we need a general way of handling that: #477

@hunner
Copy link
Contributor

hunner commented Dec 18, 2013

@apenney
Copy link
Contributor Author

apenney commented Dec 18, 2013

@igalic I just want to make sure you know I completely agree that we need a general way to handle this stuff, I'm just trying to do the absolute minimum I can to get a fully passing test suite in the short term for our internal needs. ::vhost in general needs some serious work on generalized reusable ways to do stuff.

@anyone-can-test
Copy link
Contributor

@hunner I added the explanation about index_options, index_order_default with codes which are issues here: 02283f6. So, if deleted, I am not sure if IndexOptions, IndexOrderDefault work. I am so novice.

@hunner
Copy link
Contributor

hunner commented Jan 23, 2014

@anyone-can-test Oh okay. The index_options and index_order_default values are actually passed as keys in the directories parameter hash rather than a direct parameter. So

apache::vhost { 'myhost':
  #...
  directories => [
    {
      'path'          => '/srv/web',
      'index_options' => 'IgnoreCase',
    },
  ],
}

only contains one apache::vhost parameter (directories).

hunner added a commit that referenced this pull request Jan 23, 2014
Pass this through to directories.
@hunner hunner merged commit 523dc0c into puppetlabs:master Jan 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants