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

Extend functionality of the cassandra::schema::user class. #387

Open
cparadal opened this issue May 18, 2017 · 10 comments
Open

Extend functionality of the cassandra::schema::user class. #387

cparadal opened this issue May 18, 2017 · 10 comments
Assignees
Labels
enhancement New feature or request question
Milestone

Comments

@cparadal
Copy link

cparadal commented May 18, 2017

Hi,

We are using tag 1.25.2 and would like to add some modifications to the cassandra::schema::user class.

As it happens, we are currently in the process of testing managing Cassandra users and passwords using Hiera (hiera-eyaml for encrypted passwords) in conjunction with the cassandra::schema::user class.

The below shortcomings have been identified during the process:

  • The class only manages user creation or deletion, it's currently impossible to just modify a user password and enabling/disabling the superuser flag for a specific (existing) user.
  • If authentication is enabled in the cluster and a user's password is undefined, the create user command fails, as Cassandra is expecting the "create user if not exists with password '' syntax. Obviously, if auth is enabled, users should have passwords assigned to them, but maybe that could be worked around by assigning a blank password?).

We tested modified user.pp where an "alter user" is added, to deal with updating passwords and superuser/nosuperuser flags for existing users through Hiera.
Since Cassandra won't let you change the superuser flag for the user you have logged in with (cqlsh), some extra code was needed to prevent that.

Could you please check the below modifications to cassandra::schema::user.pp and let us know your thoughts?

# cassandra::schema::user
define cassandra::schema::user (
  $ensure          = present,
  $password        = undef,
  $superuser       = false,
  $user_name       = $title,
  ){
  include 'cassandra::schema'
  $read_script = 'LIST USERS'
  $read_command = "${::cassandra::schema::cqlsh_opts} -e \"${read_script}\" ${::cassandra::schema::cqlsh_conn} | grep '\s*${user_name} |'"

  if $ensure == present {
    $create_script1 = "CREATE USER IF NOT EXISTS ${user_name}"

    if $password != undef {
      $create_script2  = "${create_script1} WITH PASSWORD '${password}'"
      $alter_script1   = "ALTER USER ${user_name} WITH PASSWORD '${password}'"
    } else {
      $create_script2  = "${create_script1} WITH PASSWORD ''"
      $alter_script1   = "ALTER USER ${user_name} WITH PASSWORD ''"
    }

    if $superuser and $user_name != $::cassandra::schema::cqlsh_user {
      $create_script = "${create_script2} SUPERUSER"
      $alter_script  = "${alter_script1} SUPERUSER"
    } elsif $superuser and $user_name == $::cassandra::schema::cqlsh_user {
      $create_script = "${create_script2} SUPERUSER"
      $alter_script  = $alter_script1
    } elsif !$superuser and $user_name != $::cassandra::schema::cqlsh_user {
      $create_script = "${create_script2} NOSUPERUSER"
      $alter_script  = "${alter_script1} NOSUPERUSER"
    } elsif !$superuser and $user_name == $::cassandra::schema::cqlsh_user {
      $create_script = "${create_script2} NOSUPERUSER"
      $alter_script  = $alter_script1
    }

    $create_command = "${::cassandra::schema::cqlsh_opts} -e \"${create_script}\" ${::cassandra::schema::cqlsh_conn}"
    $alter_command  = "${::cassandra::schema::cqlsh_opts} -e \"${alter_script}\" ${::cassandra::schema::cqlsh_conn}"

    exec { "Create user (${user_name})":
      command => $create_command,
      unless  => $read_command,
      require => Exec['::cassandra::schema connection test'],
    }

    exec { "Alter user (${user_name})":
      command => $alter_command,
      onlyif  => $read_command,
      require => Exec['::cassandra::schema connection test'],
    }


  } elsif $ensure == absent {
    $delete_script = "DROP USER ${user_name}"
    $delete_command = "${::cassandra::schema::cqlsh_opts} -e \"${delete_script}\" ${::cassandra::schema::cqlsh_conn}"

    exec { "Delete user (${user_name})":
      command => $delete_command,
      onlyif  => $read_command,
      require => Exec['::cassandra::schema connection test'],
    }
  } else {
    fail("Unknown action (${ensure}) for ensure attribute.")
  }
@dallinb
Copy link
Contributor

dallinb commented May 22, 2017

Hi,

I'll look at this, but strictly the 1.X.Y version of this module is no longer supported.

Best wishes,

Ben

@dallinb dallinb self-assigned this May 22, 2017
@dallinb
Copy link
Contributor

dallinb commented May 22, 2017

Also would you consider upgrading within the legacy (1.X.Y) branch? See locp/cassandra@1.25.2...1.27.0 for details of the later additions.

@cparadal
Copy link
Author

Hi,

Looks like the code for cassandra::schema::user defined type is more or less the same in later versions.

Maybe the best bet is for us to test using latest, instead of 1.25 and maybe contribute the code for user.pp?

Thanks.

@dallinb
Copy link
Contributor

dallinb commented May 23, 2017

You probably won't be able to put in a PR for the legacy version (which is why it's not supported at the moment). I'll look at cutting another release of 1.27.1 or 1.28.0 depending on the changes.

@cparadal
Copy link
Author

Hi,

I meant testing the latest version available, 2.4.X and contribute there.

Thanks.

@dallinb
Copy link
Contributor

dallinb commented May 23, 2017

Ah, I see. Yes that would be much better. Please node there is already a PR on that very file so I may need to do some work to get them to work together.

@dallinb
Copy link
Contributor

dallinb commented May 26, 2017

There is indeed a considerable amount of conflict to be resolved since the merging of #388. I'll also need to add some acceptance tests to this.

@dallinb dallinb added the enhancement New feature or request label May 26, 2017
@dallinb dallinb added this to the Minor Release milestone May 26, 2017
@dallinb
Copy link
Contributor

dallinb commented May 31, 2017

I was looking at merging this when I noticed that the alter command is executed when the user exists regardless of if it is required or not (please correct me if I'm wrong). That means that the code would not be idempotent. I must admit this has always been a weak point of the module.

@cparadal
Copy link
Author

cparadal commented Jun 12, 2017

Hi,

Sorry for the delay in getting back to you.

Yes, I added the code with the "alter user" to be executed on every Puppet run, otherwise the password won't get changed for the Cassandra user, despite being modified on Hiera.

As the user.pp define type currently is, the password only gets set on user creation and there is no support to change/manage it afterwards.

Granted, not the most elegant solution, as the alter is always run, even if the password in Hiera is not changed... but it guarantees the password for the Cassandra user is always in line with what's in Hiera Eyaml.

@dallinb
Copy link
Contributor

dallinb commented Jun 22, 2017

I think that you have raised a valid point and that it should be possible to be able to alter not only users but other resources created by this module. Unfortunately this won't work at the moment as it would fail the acceptance tests. This is because currently the policy is to have the module be idempotent. This could be achieved by implementing these resources as custom types. Possibly will be implemented now that this module is being migrated to Vox Pupuli (see #394).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question
Projects
None yet
Development

No branches or pull requests

2 participants