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

work with token and direct compliance server API #20

Merged
merged 7 commits into from Apr 21, 2016

Conversation

Projects
None yet
4 participants
@srenatus
Collaborator

srenatus commented Apr 6, 2016

This adds the ability to directly talk to a compliance server, without
passing through a chef-server. To ensure proper authentication, this
needs a user (access) token:

token = '....'

# iterate over all selected profiles
node['audit']['profiles'].each do |owner_profile, enabled|
  next unless enabled
  o, p = owner_profile.split('/')

  compliance_profile p do
    owner o
    server URI.parse('http://192.168.33.1:8080/api/')
    token token
    action [:fetch, :execute]
  end
end

compliance_report 'chef-server' do
  server URI.parse('http://192.168.33.1:8080/api/')
  token token
  organization 'brewinc'
end if node['audit']['profiles'].values.any?

There're two big buts here, though:

  1. It's expecting an access token; so to be of real use in automation setups, something surrounding this chef-run needs to exchange a refresh token for an access token and use ENV['ACCESS_TOKEN'] (or similar) to pass it into the recipe.
  2. Compliance will only accept reports for users (identified via the token) that are authenticated using an oc_id connector (i.e., coming "from a chef server") -- and thus this doesn't make testing the audit-cookbook <--> compliance interaction that much easier: it still needs a chef server in the mix.

@iennae iennae added the in progress label Apr 6, 2016

@srenatus srenatus added the enhancement label Apr 6, 2016

@srenatus

This comment has been minimized.

Collaborator

srenatus commented Apr 7, 2016

example recipe

token = 'eyJhbGciOiJSUzI1NiIsImtpZCI6InVQUExHVjFiVDg4YTNnZ0MtQ1hxUDdfTlgzTGZiblZSeHhYUTQwV2l0cXI0Z0pRTGw3T2........'

# iterate over all selected profiles
node['audit']['profiles'].each do |owner_profile, enabled|
  next unless enabled
  o, p = owner_profile.split('/')

  compliance_profile p do
    owner o
    server URI.parse('http://192.168.33.1:8080/api/') # <---
    token token # <---
    action [:fetch, :execute]
  end
end

# report the results
compliance_report 'chef-server' do
  server URI.parse('http://192.168.33.1:8080/api/')
  token token # <---
  direct true # <---
  owner 'admin' # <---
end if node['audit']['profiles'].values.any?
tf.binmode
Net::HTTP.start(url.host, url.port) do |http|
http.use_ssl = url.scheme == 'https'
http.verify_mode = OpenSSL::SSL::VERIFY_NONE # FIXME

This comment has been minimized.

@chris-rock

chris-rock Apr 11, 2016

Collaborator

could we make it an attribute and set the default to false?

This comment has been minimized.

@srenatus

srenatus Apr 11, 2016

Collaborator

I'm afraid I don't get it. It's not boolean but a string property that is nil if unset and some string if it was provided.
So currently, it's a property of a resource. You can use attributes to set it in your recipe (see here) -- I haven't had a need to touch the default recipe here.

Having a resource break its interface and reach out into node attributes doesn't seem right to me...

This comment has been minimized.

@chris-rock

chris-rock Apr 11, 2016

Collaborator

@srenatus of course we should not use node attributes directly in a library. I was thinking about making it an option to compliance profile:

compliance_profile p do
    owner o
    server URI.parse(ENV['COMPLIANCE_API'])
    insecure true
    token token
    action [:fetch, :execute]
end

Then we could use an attribute as input. I would like to make insecure to false by default. In insecure is true, we could set the proper openssl string as required.

rest = Chef::ServerAPI.new(url, Chef::Config)
rest.post(url, blob)
if token
if direct

This comment has been minimized.

@chris-rock

chris-rock Apr 11, 2016

Collaborator

instead of direct we may use :chef_server & :chef_compliance as targets with default to chef server. What do you think?

This comment has been minimized.

@srenatus

srenatus Apr 11, 2016

Collaborator

I've changed it to variant. Note that this is not what triggers the endpoint, but only the way it is used in compliance: should it use a chef-server connector token and setup a proper chef-source for the node + env (:chef), or is it merely a non-chef-connector token (:compliance) and the stuff should be owned by the owner that is given via the other property.

If it's using the chef-gate API or directly calls out to a compliance API endpoint is determined by the presence of the token property.

@chris-rock

This comment has been minimized.

Collaborator

chris-rock commented Apr 11, 2016

@srenatus very good, I like the improvement. Just some minor questions

@srenatus

This comment has been minimized.

Collaborator

srenatus commented Apr 13, 2016

Thanks @alexpop for finding the nasty bug resolved in bfeb004 👍

@chris-rock is this what you had in mind?

@srenatus

This comment has been minimized.

Collaborator

srenatus commented Apr 18, 2016

Rebased & refactored.

@chris-rock

View changes

recipes/default.rb Outdated
@@ -17,16 +17,26 @@
# See the License for the specific language governing permissions and
# limitations under the License.
token = node['audit']['api_token']

This comment has been minimized.

@chris-rock

chris-rock Apr 18, 2016

Collaborator

Should this be token = node['audit']['token'] ?

@chris-rock

View changes

attributes/default.rb Outdated
@@ -16,6 +16,10 @@
# limitations under the License.
#
default['audit']['server'] = nil
default['audit']['api_token'] = nil

This comment has been minimized.

@chris-rock

chris-rock Apr 18, 2016

Collaborator

Isn't this the default['audit']['token']?

@chris-rock

View changes

.kitchen.yml Outdated
audit:
variant: compliance
server: <%= ENV['COMPLIANCE_API'] %>
token: <%= ENV['COMPLIANCE_TOKEN'] %>

This comment has been minimized.

@chris-rock

chris-rock Apr 18, 2016

Collaborator

I would like to use token: <%= ENV['COMPLIANCE_ACCESSTOKEN'] %> to make clear, that we are not accepting refresh tokens here

@chris-rock

This comment has been minimized.

Collaborator

chris-rock commented Apr 18, 2016

Awesome @srenatus, I just found some minor nitpicking things :-)

@chris-rock

This comment has been minimized.

Collaborator

chris-rock commented Apr 18, 2016

I merge it, once its green

@chris-rock

This comment has been minimized.

Collaborator

chris-rock commented Apr 21, 2016

Awesome @srenatus

@chris-rock chris-rock merged commit 54e178b into master Apr 21, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@chris-rock chris-rock deleted the sr/direct-connection branch Apr 21, 2016

@iennae iennae removed the in progress label Apr 21, 2016

@cjohannsen81

This comment has been minimized.

Contributor

cjohannsen81 commented May 21, 2016

Hey, just learned that there has to be a slash at the end (URI object). Otherwise it will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment