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.

Sign up for GitHub

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

Jump to bottom

docker.containers: Ensure .labels returns an Array #3673

Merged
merged 4 commits into from
Dec 20, 2018
Merged

docker.containers: Ensure .labels returns an Array #3673

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
836099a
Fix `labels` on Docker containers
jerryaldrichiii Dec 18, 2018
0c61318
Respond to feedback
jerryaldrichiii Dec 18, 2018
3ef82f6
Fix `docker_container` resource
jerryaldrichiii Dec 19, 2018
d72b65d
Add test for `docker_container` with no labels
jerryaldrichiii Dec 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/resources/docker.md.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ Or execute the profile directly via URL:
its('commands') { should_not include '/bin/sh' }
its('images') { should_not include 'u12:latest' }
its('ports') { should include '0.0.0.0:1234->1234/tcp' }
its('labels') { should include 'License=GPLv2,Vendor=CentOS' }
its('labels') { should include 'License=GPLv2' }
end

### object('id')
Expand Down
19 changes: 13 additions & 6 deletions lib/resources/docker.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class DockerContainerFilter
filter.register_column(:commands, field: 'command')
.register_column(:ids, field: 'id')
.register_column(:images, field: 'image')
.register_column(:labels, field: 'labels')
.register_column(:labels, field: 'labels', style: :simple)
.register_column(:local_volumes, field: 'localvolumes')
.register_column(:mounts, field: 'mounts')
.register_column(:names, field: 'names')
Expand Down Expand Up @@ -190,21 +190,28 @@ def parse_json_command(labels, subcommand)
# since docker is not outputting valid json, we need to parse each row
raw.each_line { |entry|
# convert all keys to lower_case to work well with ruby and filter table
j = JSON.parse(entry).map { |k, v|
[k.downcase, v]
row = JSON.parse(entry).map { |key, value|
[key.downcase, value]
}.to_h

# ensure all keys are there
j = ensure_keys(j, labels)
row = ensure_keys(row, labels)

# strip off any linked container names
# Depending on how it was linked, the actual container name may come before
# or after the link information, so we'll just look for the first name that
# does not include a slash since that is not a valid character in a container name
j['names'] = j['names'].split(',').find { |c| !c.include?('/') } if j.key?('names')
if row['names']
row['names'] = row['names'].split(',').find { |c| !c.include?('/') }
end

# Split labels on ',' or set to empty array
# Allows for `docker.containers.where { labels.include?('app=redis') }`
row['labels'] = row.key?('labels') ? row['labels'].split(',') : []

output.push(j)
output.push(row)
}

output
rescue JSON::ParserError => _e
warn "Could not parse `docker #{subcommand}` output"
Expand Down
1 change: 1 addition & 0 deletions lib/resources/docker_container.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class DockerContainer < Inspec.resource(1)
its('tag') { should eq 'latest' }
its('ports') { should eq [] }
its('command') { should eq 'nc -ll -p 1234 -e /bin/cat' }
its('labels') { should include 'app=example' }
end

describe docker_container(id: 'e2c52a183358') do
Expand Down
2 changes: 1 addition & 1 deletion test/unit/mock/cmd/docker-ps-a
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{"Command":"\"/bin/bash\"","CreatedAt":"2017-04-24 10:29:12 +0200 CEST","ID":"3def9aa450f8bd772c3d5b07e27ec934e5f58575e955367a0aca2d93e0687536","Image":"ubuntu:12.04","Labels":"","LocalVolumes":"0","Mounts":"","Names":"sleepy_khorana","Networks":"bridge","Ports":"","RunningFor":"29 minutes","Size":"0 B","Status":"Exited (127) 2 seconds ago"}
clintoncwolfe marked this conversation as resolved.
Show resolved Hide resolved
{"Command":"\"/bin/bash\"","CreatedAt":"2017-04-24 10:29:12 +0200 CEST","ID":"3def9aa450f8bd772c3d5b07e27ec934e5f58575e955367a0aca2d93e0687536","Image":"ubuntu:12.04","Labels":"app=example,version=1.5.4","LocalVolumes":"0","Mounts":"","Names":"sleepy_khorana","Networks":"bridge","Ports":"","RunningFor":"29 minutes","Size":"0 B","Status":"Exited (127) 2 seconds ago"}
{"Command":"\"/bin/sh\"","CreatedAt":"2017-04-22 22:44:42 +0200 CEST","ID":"d94f854370d2b02912e8fc636502bc72b74fbd567a7eba3fc6a52045bb28904e","Image":"alpine","Labels":"","LocalVolumes":"0","Mounts":"","Names":"laughing_austin,sleepy_khorana/container1","Networks":"bridge","Ports":"","RunningFor":"36 hours","Size":"0 B","Status":"Exited (0) 35 hours ago"}
{"Command":"\"/bin/sh\"","CreatedAt":"2017-08-03 12:56:03 +0200 CEST","ID":"5a83c301f30ccd48579a74a84af6fdd0c0e0d66aacc7bb52abfa2ba2544c6c0c","Image":"repo.example.com:5000/ubuntu:14.04","Labels":"","LocalVolumes":"0","Mounts":"","Names":"heuristic_almeida","Networks":"bridge","Ports":"","RunningFor":"5 hours","Size":"0 B","Status":"Exited (0) 24 hours ago"}
{"Command":"\"/bin/sh\"","CreatedAt":"2017-08-03 12:56:03 +0200 CEST","ID":"5a83c301f30ccd48579a74a84af6fdd0c0e0d66aacc7bb52abfa2ba2544c6c0c","Image":"repo.example.com:5000/ubuntu","Labels":"","LocalVolumes":"0","Mounts":"","Names":"sleepy_khorana/container1,laughing_austin/container2,laughing_lamport","Networks":"bridge","Ports":"","RunningFor":"5 hours","Size":"0 B","Status":"Exited (0) 24 hours ago"}
1 change: 1 addition & 0 deletions test/unit/resources/docker_container_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
_(resource.tag).must_equal '12.04'
_(resource.command).must_equal '/bin/bash'
_(resource.ports).must_equal ''
_(resource.labels).must_equal ['app=example', 'version=1.5.4']
Copy link
Contributor

Choose a reason for hiding this comment

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

This currently has a failing unit test, which needs to be fixed.

Also, you mentioned that there was an existing unit test which tested a container with zero labels; could you show me where that is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, looks like I needed to fix the docker_container resource since labels[0] is no longer correct.

Also, I didn't see where a test where a docker container was explicitly returning [] when no labels are used. Couldn't hurt to have an explicit test instead of relying on docker.containers.labels being correct. I'll make it so.

end

it 'check image containing repo with port and tag gives correct repo, image, and tag' do
Expand Down
1 change: 1 addition & 0 deletions test/unit/resources/docker_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
it 'check docker container parsing' do
_(resource.containers.ids).must_equal ['3def9aa450f8bd772c3d5b07e27ec934e5f58575e955367a0aca2d93e0687536', 'd94f854370d2b02912e8fc636502bc72b74fbd567a7eba3fc6a52045bb28904e', '5a83c301f30ccd48579a74a84af6fdd0c0e0d66aacc7bb52abfa2ba2544c6c0c', '5a83c301f30ccd48579a74a84af6fdd0c0e0d66aacc7bb52abfa2ba2544c6c0c']
_(resource.containers.names).must_equal ['sleepy_khorana', 'laughing_austin', 'heuristic_almeida', 'laughing_lamport']
_(resource.containers.labels).must_equal ['app=example', 'version=1.5.4']
end

it 'check docker image parsing' do
Expand Down