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

mysql_grant fixed to properly handle PROCEDURE grants #412

Merged
merged 2 commits into from
Jan 22, 2014

Conversation

dgolja
Copy link
Contributor

@dgolja dgolja commented Jan 11, 2014

Bugfix for mysql_grant provider when we try to grant privileges on PROCEDURE.

Resolve bug https://tickets.puppetlabs.com/browse/MODULES-130 (#378)

@igalic
Copy link
Contributor

igalic commented Jan 13, 2014

Could you please also add this test to spec/acceptance?

@dgolja
Copy link
Contributor Author

dgolja commented Jan 13, 2014

So you want me to move the test from spec/system/types/ to spec /acceptance/ ?

@igalic
Copy link
Contributor

igalic commented Jan 14, 2014

Essentially, yes. We're going to ax spec/system soon.

@dgolja
Copy link
Contributor Author

dgolja commented Jan 17, 2014

done ...

@igalic
Copy link
Contributor

igalic commented Jan 17, 2014

Cool. Could you squash those down to two? (i.e.: We don't care about the merge from remote.. etc..)
I'll do a test-run now and merge later

@igalic
Copy link
Contributor

igalic commented Jan 17, 2014

or not :S

igalic@levix ~/src/puppet/puppetlabs-mysql (git)-[REQUIRE] % git pulls merge 384
git merge --no-ff -m 'Merge pull request #384 from srinathman/puppetlabs-mysql

---

Added the option add REQUIRE clause to the grant command. This allow SSL based authentication.
http://dev.mysql.com/doc/refman/5.5/en/grant.html

Example:
```puppet
mysql_grant { user1@host1/*.*":
  ensure => present,
  options => [GRANT],
  privileges => [ALL],
  table => *.*,
  user => user1@host1,
  grant_require => [SUBJECT "/CN=user1/OU=Company", ISSUER "CompanyCA" ] ,
}

generates:

GRANT ALL PRIVILEGES ON *.* TO user1@host1 REQUIRE ISSUER CompanyCA SUBJECT /CN=user1/OU=Company WITH GRANT OPTION
```' 7f657b8080bc61e8bccc2e3ec69e1212888d45ee
Auto-merging lib/puppet/type/mysql_grant.rb
Auto-merging lib/puppet/provider/mysql_grant/mysql.rb
CONFLICT (content): Merge conflict in lib/puppet/provider/mysql_grant/mysql.rb
Automatic merge failed; fix conflicts and then commit the result.
igalic@levix ~/src/puppet/puppetlabs-mysql (git)-[REQUIRE|merge] % git status
# On branch REQUIRE
# You have unmerged paths.
#   (fix conflicts and run "git commit")
#
# Changes to be committed:
#
#       modified:   lib/puppet/provider/mysql.rb
#       modified:   lib/puppet/type/mysql_grant.rb
#
# Unmerged paths:
#   (use "git add <file>..." to mark resolution)
#
#       both modified:      lib/puppet/provider/mysql_grant/mysql.rb
#
igalic@levix ~/src/puppet/puppetlabs-mysql (git)-[REQUIRE|merge] % git diff
diff --cc lib/puppet/provider/mysql_grant/mysql.rb
index 7dabf46,0170596..0000000
--- lib/puppet/provider/mysql_grant/mysql.rb
+++ lib/puppet/provider/mysql_grant/mysql.rb
@@@ -22,7 -22,11 +22,15 @@@ Puppet::Type.type(:mysql_grant).provide
              priv == 'ALL PRIVILEGES' ? 'ALL' : priv.lstrip.rstrip
            end
            # Same here, but to remove OPTION leaving just GRANT.
++<<<<<<< HEAD
 +          options = ['GRANT'] if rest.match(/WITH\sGRANT\sOPTION/)
++=======
+           options = rest.match(/WITH\s(.*)\sOPTION$/).captures if rest.include?('WITH')
+ 
+           # Get all REQUIRE options.
+           grant_require = rest.match(/REQUIRE\s(.*)\sWITH/).captures if rest.include?('REQUIRE')
+ 
++>>>>>>> 7f657b8080bc61e8bccc2e3ec69e1212888d45ee
            # We need to return an array of instances so capture these
            instances << new(
                :name       => "#{user}@#{host}/#{table}",
igalic@levix ~/src/puppet/puppetlabs-mysql (git)-[REQUIRE|merge] %

@dgolja
Copy link
Contributor Author

dgolja commented Jan 17, 2014

Didn't you try to merge the wrong PR ? (384 instead of 412)

@igalic
Copy link
Contributor

igalic commented Jan 17, 2014

Apparently….

@dgolja
Copy link
Contributor Author

dgolja commented Jan 17, 2014

haha even the best makes mistakes :) So which one commit do you want me to squish ? Also If you want I can just create another PR with the same content :bowtie:

@igalic
Copy link
Contributor

igalic commented Jan 17, 2014

You should get rid of the 'merge' commit by rebasing from upstream, that is, putting your changes on top of upstream's.
I would suggest you squash the two commits to tests down to one.

@dgolja
Copy link
Contributor Author

dgolja commented Jan 20, 2014

done... Please have a look if that's what you were expecting.

igalic added a commit that referenced this pull request Jan 22, 2014
mysql_grant fixed to properly handle PROCEDURE grants
@igalic igalic merged commit aa3e1d6 into puppetlabs:master Jan 22, 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.

3 participants