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

backporting support for server < 12.5.0 #106

Merged
merged 3 commits into from
Aug 3, 2017

Conversation

jeremymv2
Copy link
Contributor

This addresses #103

This change was tested with backups and restores against Chef Server 12.1.0, 12.5.0 and 12.6.0

Signed-off-by: Jeremy J. Miller jm@chef.io

Signed-off-by: Jeremy J. Miller <jm@chef.io>
@jeremymv2 jeremymv2 changed the title backporting support for server <= 12.5.0 backporting support for server < 12.5.0 Jul 25, 2017
restore_group(chef_fs_config, group, :clients => false)
end

['/acls/groups/billing-admins.json', '/acls/groups/public_key_read_access.json'].each do |acl|
acls_groups_paths = ['/acls/groups/billing-admins.json']
acls_groups_paths.push('/acls/groups/public_key_read_access.json') if server.supports_public_key_read_access?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen if the backup doesn't have a public_key_read_access group? From my reading of the code we rescue that in the case of the group, but for the ACL I guess we would report it as an error at the end of the run?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks like the rescue happens then it gets reported at the end. Here's a backup of 12.1.0 with two orgs being restored to a 12.15.8:

knife ec restore backups/ --purge
Restoring users
WARNING: Skipping pivotal user.  To overwrite pivotal, pass --overwrite-pivotal.
Restoring organization[acmeinc]
Restoring org admin data
WARN: Could not find /groups/public_key_read_access.json on disk. Will not restore.
Copying /acls/groups/billing-admins.json
Copying /acls/groups/public_key_read_access.json
ERROR: /acls/groups/public_key_read_access.json ACLs cannot be deleted.
Restoring the rest of the org
Copying /clients
Copying /containers
Copying /cookbook_artifacts
Copying /cookbooks
Copying /data_bags
Copying /environments
Copying /invitations.json
Copying /members.json
Copying /nodes
Copying /org.json
Copying /policies
Copying /policy_groups
Copying /roles
Copying /groups/00000000000003a94e506949fa5629f5.json
Copying /groups/admins.json
Updated /groups/admins.json
Copying /groups/clients.json
Copying /groups/users.json
Copying /groups/00000000000003a94e506949fa5629f5.json
Copying /groups/admins.json
Updated /groups/admins.json
Copying /groups/clients.json
Copying /groups/users.json
Copying /acls/groups/00000000000003a94e506949fa5629f5.json
Copying /acls/groups/admins.json
Copying /acls/groups/clients.json
Copying /acls/groups/users.json
Copying /acls/clients
Copying /acls/containers
Copying /acls/environments
Copying /acls/organization.json
WARN: Could not find /groups/public_key_read_access.json on disk. Will not restore.
Restoring organization[brewinc]
Restoring org admin data
WARN: Could not find /groups/public_key_read_access.json on disk. Will not restore.
Copying /acls/groups/billing-admins.json
Copying /acls/groups/public_key_read_access.json
ERROR: /acls/groups/public_key_read_access.json ACLs cannot be deleted.
Restoring the rest of the org
Copying /clients
Copying /containers
Copying /cookbook_artifacts
Copying /cookbooks
Copying /data_bags
Copying /environments
Copying /invitations.json
Copying /members.json
Copying /nodes
Copying /org.json
Copying /policies
Copying /policy_groups
Copying /roles
Copying /groups/00000000000015096a9c2b30c5e076ba.json
Copying /groups/00000000000064af00f9de8836ae986c.json
Copying /groups/admins.json
Updated /groups/admins.json
Copying /groups/clients.json
Copying /groups/users.json
Copying /groups/00000000000015096a9c2b30c5e076ba.json
Copying /groups/00000000000064af00f9de8836ae986c.json
Copying /groups/admins.json
Updated /groups/admins.json
Copying /groups/clients.json
Copying /groups/users.json
Copying /acls/groups/00000000000015096a9c2b30c5e076ba.json
Copying /acls/groups/00000000000064af00f9de8836ae986c.json
Copying /acls/groups/admins.json
Copying /acls/groups/clients.json
Copying /acls/groups/users.json
Copying /acls/clients
Copying /acls/containers
Copying /acls/environments
Copying /acls/organization.json
WARN: Could not find /groups/public_key_read_access.json on disk. Will not restore.
Restoring user ACLs
WARNING: Skipping pivotal user.  To overwrite pivotal, pass --overwrite-pivotal.

Error Summary Report
{
  "timestamp": "2017-07-25 20:08:28 -0400",
  "message": "400 \"Bad Request\"",
  "backtrace": [
    "/opt/chefdk/embedded/lib/ruby/2.4.0/net/http/response.rb:122:in `error!'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/http.rb:152:in `request'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/http.rb:123:in `put'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:369:in `block in put_acl'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:368:in `each'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:368:in `put_acl'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:105:in `block in restore_user_acls'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:117:in `block in for_each_user'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:110:in `foreach'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:110:in `for_each_user'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:103:in `restore_user_acls'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:58:in `run'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/knife.rb:442:in `block in run_with_pretty_exceptions'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/local_mode.rb:44:in `with_server_connectivity'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/knife.rb:441:in `run_with_pretty_exceptions'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/knife.rb:219:in `run'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/application/knife.rb:156:in `run'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/bin/knife:25:in `<top (required)>'",
    "/opt/chefdk/bin/knife:264:in `load'",
    "/opt/chefdk/bin/knife:264:in `<main>'"
  ],
  "exception": "Net::HTTPServerException"
}{
  "timestamp": "2017-07-25 20:08:28 -0400",
  "message": "400 \"Bad Request\"",
  "backtrace": [
    "/opt/chefdk/embedded/lib/ruby/2.4.0/net/http/response.rb:122:in `error!'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/http.rb:152:in `request'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/http.rb:123:in `put'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:369:in `block in put_acl'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:368:in `each'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:368:in `put_acl'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:105:in `block in restore_user_acls'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:117:in `block in for_each_user'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:110:in `foreach'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:110:in `for_each_user'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:103:in `restore_user_acls'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:58:in `run'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/knife.rb:442:in `block in run_with_pretty_exceptions'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/local_mode.rb:44:in `with_server_connectivity'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/knife.rb:441:in `run_with_pretty_exceptions'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/knife.rb:219:in `run'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/application/knife.rb:156:in `run'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/bin/knife:25:in `<top (required)>'",
    "/opt/chefdk/bin/knife:264:in `load'",
    "/opt/chefdk/bin/knife:264:in `<main>'"
  ],
  "exception": "Net::HTTPServerException"
}{
  "timestamp": "2017-07-25 20:08:28 -0400",
  "message": "400 \"Bad Request\"",
  "backtrace": [
    "/opt/chefdk/embedded/lib/ruby/2.4.0/net/http/response.rb:122:in `error!'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/http.rb:152:in `request'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/http.rb:123:in `put'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:369:in `block in put_acl'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:368:in `each'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:368:in `put_acl'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:105:in `block in restore_user_acls'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:117:in `block in for_each_user'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:110:in `foreach'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:110:in `for_each_user'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:103:in `restore_user_acls'",
    "/Users/jmiller/.chefdk/gem/ruby/2.4.0/gems/knife-ec-backup-2.2.1/lib/chef/knife/ec_restore.rb:58:in `run'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/knife.rb:442:in `block in run_with_pretty_exceptions'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/local_mode.rb:44:in `with_server_connectivity'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/knife.rb:441:in `run_with_pretty_exceptions'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/knife.rb:219:in `run'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/application/knife.rb:156:in `run'",
    "/opt/chefdk/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/bin/knife:25:in `<top (required)>'",
    "/opt/chefdk/bin/knife:264:in `load'",
    "/opt/chefdk/bin/knife:264:in `<main>'"
  ],
  "exception": "Net::HTTPServerException"
}

Error(s) Summary file located at: 'backups//errors/restore-2017-07-25T20:08:28-04:00.json'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 the 3 put_acl HTTP 400 errors are actually coming from the restoring user acls section and it appears to only be related to the read permission:

Restoring user ACLs
PUTTING users/admin/_acl/create {"actors"=>["admin", "pivotal"], "groups"=>[]}
PUTTING users/admin/_acl/read {"actors"=>["admin", "pivotal"], "groups"=>["brewinc_global_admins"]}
PUTTING users/delivery/_acl/create {"actors"=>["delivery", "pivotal"], "groups"=>[]}
PUTTING users/delivery/_acl/read {"actors"=>["delivery", "pivotal"], "groups"=>["brewinc_global_admins"]}
PUTTING users/john/_acl/create {"actors"=>["john", "pivotal"], "groups"=>[]}
PUTTING users/john/_acl/read {"actors"=>["john", "pivotal"], "groups"=>["acmeinc_global_admins"]}
192.168.200.1 - - [26/Jul/2017:01:02:38 +0000]  "PUT /users/admin/_acl/read HTTP/1.1" 400 "0.013" 85 "-" "Chef Knife/13.2.20 (ruby-2.4.1-p111; ohai-13.2.0; x86_64-darwin14; +https://chef.io)" "127.0.0.1:8000" "400" "0.011" "13.2.20" "algorithm=sha1;version=1.1;" "pivotal" "2017-07-26T01:02:38Z" "JdmKCWPL/xK0R92iOrjVfmJGVFo=" 1181
192.168.200.1 - - [26/Jul/2017:01:02:38 +0000]  "PUT /users/delivery/_acl/read HTTP/1.1" 400 "0.008" 85 "-" "Chef Knife/13.2.20 (ruby-2.4.1-p111; ohai-13.2.0; x86_64-darwin14; +https://chef.io)" "127.0.0.1:8000" "400" "0.007" "13.2.20" "algorithm=sha1;version=1.1;" "pivotal" "2017-07-26T01:02:38Z" "WVBLmVwVGSufar2rlHG7Qjnnk4s=" 1187
192.168.200.1 - - [26/Jul/2017:01:02:38 +0000]  "PUT /users/john/_acl/read HTTP/1.1" 400 "0.011" 85 "-" "Chef Knife/13.2.20 (ruby-2.4.1-p111; ohai-13.2.0; x86_64-darwin14; +https://chef.io)" "127.0.0.1:8000" "400" "0.008" "13.2.20" "algorithm=sha1;version=1.1;" "pivotal" "2017-07-26T01:02:38Z" "+PWx9k+Z+PaRwd3FXA3yRGBlINY=" 1179

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can circumvent the other WARN and ERROR messages for public_key_read_access.json by adding a simple filesystem check that it exists:

::File.exist?(::File.join(chef_fs_config.local_fs.child_paths['acls'], 'groups', 'public_key_read_access.json'))

But I'm wondering what's going on with the user acls and how that hasn't been discovered before..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh it's likely because the rename that happened in Server 12.2.0 $ORG_global_admins to $ORG_read_access_group -> chef/chef-server@0172409

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevendanna high-level thoughts on how to handle that? Address here or in a new PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user-acl changes from this in 12.1.0

  "read": {
    "actors": [
      "pivotal",
      "delivery"
    ],
    "groups": [
      "brewinc_global_admins"
    ]
  },

To this in 12.15.8

  "read": {
    "actors": [
      "delivery",
      "pivotal"
    ],
    "groups": [
      "::brewinc_read_access_group",
      "::server-admins"
    ]
  },

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeremymv2

So, some of these issues we can fix and others we can't. It likely isn't /just/ because of the naming change.

Older versions of Chef Server have no way to reference "global groups". Because of this, they will never be able to upload a user acl since the user-acl contains global groups. However, depending on what your backup has, this usually isn't a problem since most people don't touch user ACLs and we check if the user-acl needs to be updated before attempting to update it. Since the user ACL gets created when we create the user, for many backups of older servers, the version on disk matched the newly created user acl.

Unfortunately, the addition of the ::server-admins group and the renaming of the globa_admins groups mean that many more people now have user acls in their backups that don't match the version that the server will create by default.

Here is what I think a full solution would look like:

  1. If we are restoring to a version of Chef Server that doesn't support global groups in ACL updates, skip updating any user ACLs that have global groups in them (this is probably ALL user ACLs).

  2. If we are restoring to a version of Chef Server that DOES support global groups in ACL updates:

a) Replace instances of ORGNAME_global_admins with ORGNAME_read_access_group before uploading UNLESS ORGNAME_global_admins /also/ exists as a group inside that organizations.

b) Compare the newly created ACL on the server to the version in the backup. If the user ACL doesn't contain a ::server-admins group but the server supports server-admins, we should probably (perhaps as an option) add this to the ACL.

If after (a) and (b) the ACLs are the same, we can skip the upload entirely. Some of this would be made easier if the backups included a VERSION file that told us which version of the chef-server a backup came from.

It isn't going to be 100% clean in any direction unfortunately.

Copy link
Contributor Author

@jeremymv2 jeremymv2 Jul 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevendanna I've linked your comment above and the idea of a VERSION file in #107

In terms of #103, do you think this PR addresses it sufficiently?

Signed-off-by: Jeremy J. Miller <jm@chef.io>
@jeremymv2
Copy link
Contributor Author

Testing removing the public_key_read_access groups and acls files from the backup then doing a restore:

$ rm backups2/organizations/*/acls/groups/public_key_read_access.json
$ rm backups2/organizations/*/groups/public_key_read_access.json
$ knife ec restore backups2/ --webui-key webui_priv.pem -c knife.rb --purge
Restoring users
WARNING: Skipping pivotal user.  To overwrite pivotal, pass --overwrite-pivotal.
Restoring organization[acmeinc]
Restoring org admin data
Copying /groups/admins.json
Copying /groups/billing-admins.json
Copying /acls/groups/billing-admins.json
Restoring the rest of the org
Copying /clients
Copying /containers
Copying /cookbook_artifacts
Copying /cookbooks
Copying /data_bags
Copying /environments
Copying /invitations.json
Copying /members.json
Copying /nodes
Copying /org.json
Copying /policies
Copying /policy_groups
Copying /roles
Copying /groups/00000000000003a94e506949fa5629f5.json
Copying /groups/admins.json
Copying /groups/clients.json
Copying /groups/users.json
Copying /groups/00000000000003a94e506949fa5629f5.json
Copying /groups/admins.json
Copying /groups/clients.json
Copying /groups/users.json
Copying /acls/groups/00000000000003a94e506949fa5629f5.json
Copying /acls/groups/admins.json
Copying /acls/groups/clients.json
Copying /acls/groups/users.json
Copying /acls/clients
Copying /acls/containers
Copying /acls/environments
Copying /acls/organization.json
Copying /groups/admins.json
Copying /groups/billing-admins.json
Restoring organization[brewinc]
Restoring org admin data
Copying /groups/admins.json
Copying /groups/billing-admins.json
Copying /acls/groups/billing-admins.json
Restoring the rest of the org
Copying /clients
Copying /containers
Copying /cookbook_artifacts
Copying /cookbooks
Copying /data_bags
Copying /environments
Copying /invitations.json
Copying /members.json
Copying /nodes
Copying /org.json
Copying /policies
Copying /policy_groups
Copying /roles
Copying /groups/00000000000015096a9c2b30c5e076ba.json
Copying /groups/00000000000064af00f9de8836ae986c.json
Copying /groups/admins.json
Copying /groups/clients.json
Copying /groups/users.json
Copying /groups/00000000000015096a9c2b30c5e076ba.json
Copying /groups/00000000000064af00f9de8836ae986c.json
Copying /groups/admins.json
Copying /groups/clients.json
Copying /groups/users.json
Copying /acls/groups/00000000000015096a9c2b30c5e076ba.json
Copying /acls/groups/00000000000064af00f9de8836ae986c.json
Copying /acls/groups/admins.json
Copying /acls/groups/clients.json
Copying /acls/groups/users.json
Copying /acls/clients
Copying /acls/containers
Copying /acls/environments
Copying /acls/organization.json
Copying /groups/admins.json
Copying /groups/billing-admins.json
Restoring user ACLs
WARNING: Skipping pivotal user.  To overwrite pivotal, pass --overwrite-pivotal.

Copy link
Contributor

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left an inline question, but this looks like the right direction to me.

restore_group(chef_fs_config, group, :clients => false)
end

['/acls/groups/billing-admins.json', '/acls/groups/public_key_read_access.json'].each do |acl|
acls_groups_paths = ['/acls/groups/billing-admins.json']
if server.supports_public_key_read_access? &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should skip the version check on restore here since, if the group is in the backup it could potentially appear in ACLs which would then fail to upload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevendanna Ok, I can get behind that. I think another way of looking at this is, in the context of restores, if the file exists in the backup, then copy it to the Chef Server by golly. It makes the logical assumption that the user is restoring to an equal or greater version of Chef Server.

In that case, remove the server.supports_public_key_read_access? check for both the group and the acls_group and just rely on the ::File.exist? check??

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for "by golly"

Signed-off-by: Jeremy J. Miller <jm@chef.io>
@jeremymv2 jeremymv2 requested a review from itmustbejj July 28, 2017 01:21
@itmustbejj
Copy link
Contributor

👍 This looks great, I've been bitten by this more than once.

Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me (haven't tried it, though)

@jeremymv2 jeremymv2 merged commit cc61bb5 into chef:master Aug 3, 2017
@jeremymv2 jeremymv2 deleted the jeremymv2/public_key_read_access branch August 3, 2017 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants