Do not strip port for insecure check by baude · Pull Request #1012 · projectatomic/atomic

baude · 2017-05-23T14:50:45Z

Description

When checking if a registry is insecure, we should not be stripping
the port from the registry name.

Related Bugzillas

#1453100

Related Issue Numbers

Pull Request Checklist:

If your Pull request contains new features or functions, tests are required. If the PR is a bug fix and no tests exist, please consider adding some to prevent regressions.

Unittests
Integration Tests

When checking if a registry is insecure, we should not be stripping the port from the registry name.

runcom · 2017-05-23T14:58:27Z

nice 👍

miabbott · 2017-05-23T15:06:41Z

When the registry is specified as insecure in /etc/sysconfig/docker, this PR works fine:

# ./atomic pull brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2
Pulling brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 ...
WARN[0000] '--tls-verify' is deprecated, please set this on the specific subcommand 
Copying blob sha256:51baebb1fefa46091f94bde51129fdb997a4246e1109d59e31cd2103235a35cc
 69.56 MB / 70.50 MB [=========================================================]
Copying blob sha256:4652ee8dcd2bfc7fc7de7aa1cb94373a8c6a55507b1bc35bcefc6475f321c3c2
 0 B / 1.17 KB [---------------------------------------------------------------]
Copying blob sha256:0ad936665f779b6ceeb0bde9edc16281122d20688d026975840c5b5d4923767f
 3.23 MB / 5.63 MB [=================================>-------------------------]
Copying config sha256:aa7cb82717fe3f689c38305cbd29349bfdb0b6d8160046ce1ccc0e01095cd9db
 0 B / 5.97 KB [---------------------------------------------------------------]
Writing manifest to image destination
Storing signatures
 5.97 KB / 5.97 KB [===========================================================]

However, if the registry is not specified as insecure in /etc/sysconfig/docker, the user gets an error that is alarming:

# ./atomic pull brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2
Pulling brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 ...
FATA[0000] runtime error: invalid memory address or nil pointer dereference 

# ./atomic --debug pull brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2                                                                                 
Namespace(_class=<class 'Atomic.pull.Pull'>, assumeyes=False, debug=True, func='pull_image', ignore=False, image='brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2', profile=False, reg_type=None, storage=None)
Pulling brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 ...
Executing: /usr/bin/skopeo --policy=/etc/containers/policy.json --debug copy --remove-signatures docker://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 docker-daemon:brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2
DEBU[0000] Using registries.d directory /etc/containers/registries.d for sigstore configuration 
DEBU[0000]  Using "default-docker" configuration        
DEBU[0000]  No signature storage configuration found for brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 
DEBU[0000] IsRunningImageAllowed for image docker:brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 
DEBU[0000]  Using default policy section                
DEBU[0000]  Requirement 0: allowed                      
DEBU[0000] Overall: allowed                             
DEBU[0000] GET https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v2/ 
DEBU[0000] Ping https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v2/ err &url.Error{Op:"Get", URL:"https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v2/", Err:(*errors.errorString)(0xc420011780)} 
DEBU[0000] GET https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v1/_ping 
DEBU[0000] Ping https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v1/_ping err &url.Error{Op:"Get", URL:"https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v1/_ping", Err:(*errors.errorString)(0xc420011b30)} 
DEBU[0000] docker-daemon: Closing tar stream to abort loading 
FATA[0000] runtime error: invalid memory address or nil pointer dereference 

Traceback (most recent call last):
  File "./atomic", line 203, in <module>
    sys.exit(_func())
  File "/root/atomic/Atomic/pull.py", line 54, in pull_image
    be.pull_image(self.args.image, remote_image_obj, debug=self.args.debug, assumeyes=self.args.assumeyes)
  File "/root/atomic/Atomic/backends/_docker.py", line 344, in pull_image
    policy_filename=trust.policy_filename)
  File "/root/atomic/Atomic/util.py", line 426, in skopeo_copy
    return check_call(cmd, env=os.environ)
  File "/root/atomic/Atomic/util.py", line 197, in check_call
    return subprocess.check_call(cmd, env=env, stdin=stdin, stderr=stderr, stdout=stdout, close_fds=True)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
CalledProcessError: Command '['/usr/bin/skopeo', '--policy=/etc/containers/policy.json', '--debug', 'copy', '--remove-signatures', 'docker://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2', 'docker-daemon:brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2']' returned non-zero exit status 1

Not sure if you want to catch that error and clean it up or just plow ahead with this PR.

miabbott · 2017-05-23T15:31:36Z

@runcom informed me that the error above from skopeo is a panic. Seen using skopeo-0.1.19-1.dev.git0224d8c.fc25.x86_64

When repeating the same atomic pull using git master from containers/skopeo@e1884aa, the panic did not occur and instead we see:

# ./atomic --debug pull brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2
Namespace(_class=<class 'Atomic.pull.Pull'>, assumeyes=False, debug=True, func='pull_image', ignore=False, image='brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2', profile=False, reg_type=None, storage=None)
Pulling brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 ...
Executing: /usr/bin/skopeo --policy=/etc/containers/policy.json --debug copy --remove-signatures docker://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 docker-daemon:brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2
DEBU[0000] Using registries.d directory /etc/containers/registries.d for sigstore configuration 
DEBU[0000]  Using "default-docker" configuration        
DEBU[0000]  No signature storage configuration found for brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 
DEBU[0000] IsRunningImageAllowed for image docker:brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2 
DEBU[0000]  Using default policy section                
DEBU[0000]  Requirement 0: allowed                      
DEBU[0000] Overall: allowed                             
DEBU[0000] GET https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v2/ 
DEBU[0000] Ping https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v2/ err &url.Error{Op:"Get", URL:"https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v2/", Err:(*errors.errorString)(0xc4203f63c0)} 
DEBU[0000] GET https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v1/_ping 
DEBU[0000] Ping https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v1/_ping err &url.Error{Op:"Get", URL:"https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v1/_ping", Err:(*errors.errorString)(0xc4203f6d00)} 
DEBU[0000] docker-daemon: Closing tar stream to abort loading 
FATA[0000] Error initializing image from source docker://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2: pinging docker registry returned: Get https://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/v2/: http: server gave HTTP response to HTTPS client 

Traceback (most recent call last):
  File "./atomic", line 203, in <module>
    sys.exit(_func())
  File "/root/atomic/Atomic/pull.py", line 54, in pull_image
    be.pull_image(self.args.image, remote_image_obj, debug=self.args.debug, assumeyes=self.args.assumeyes)
  File "/root/atomic/Atomic/backends/_docker.py", line 344, in pull_image
    policy_filename=trust.policy_filename)
  File "/root/atomic/Atomic/util.py", line 426, in skopeo_copy
    return check_call(cmd, env=os.environ)
  File "/root/atomic/Atomic/util.py", line 197, in check_call
    return subprocess.check_call(cmd, env=env, stdin=stdin, stderr=stderr, stdout=stdout, close_fds=True)
  File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
CalledProcessError: Command '['/usr/bin/skopeo', '--policy=/etc/containers/policy.json', '--debug', 'copy', '--remove-signatures', 'docker://brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2', 'docker-daemon:brew-pulp-docker01.web.prod.ext.phx2.redhat.com:8888/rhel7/cockpit-ws:138-2']' returned non-zero exit status 1

runcom · 2017-05-23T15:33:43Z

@miabbott skopeo-0.1.20 and skopeo-0.1.21 should come with the fix already, not sure they've been build for Fedora/RHEL for you to test though.

rhatdan · 2017-05-23T17:41:27Z

@rh-atomic-bot r+ ef9fb1b

rh-atomic-bot · 2017-05-23T17:41:30Z

⌛ Testing commit ef9fb1b with merge 324ea57...

TomSweeneyRedHat

LGTM

rh-atomic-bot · 2017-05-23T17:59:18Z

☀️ Test successful - status-redhatci
Approved by: rhatdan
Pushing 324ea57 to master...

Do not strip port for insecure check

ef9fb1b

When checking if a registry is insecure, we should not be stripping the port from the registry name.

TomSweeneyRedHat reviewed May 23, 2017

View reviewed changes

rh-atomic-bot closed this in 324ea57 May 23, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not strip port for insecure check#1012

Do not strip port for insecure check#1012
baude wants to merge 1 commit intoprojectatomic:masterfrom
baude:insecure_keep_port

baude commented May 23, 2017

Uh oh!

runcom commented May 23, 2017

Uh oh!

miabbott commented May 23, 2017

Uh oh!

miabbott commented May 23, 2017

Uh oh!

runcom commented May 23, 2017

Uh oh!

rhatdan commented May 23, 2017

Uh oh!

rh-atomic-bot commented May 23, 2017

Uh oh!

TomSweeneyRedHat left a comment

Uh oh!

rh-atomic-bot commented May 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

baude commented May 23, 2017

Description

Related Bugzillas

Related Issue Numbers

Pull Request Checklist:

Uh oh!

runcom commented May 23, 2017

Uh oh!

miabbott commented May 23, 2017

Uh oh!

miabbott commented May 23, 2017

Uh oh!

runcom commented May 23, 2017

Uh oh!

rhatdan commented May 23, 2017

Uh oh!

rh-atomic-bot commented May 23, 2017

Uh oh!

TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

Uh oh!

rh-atomic-bot commented May 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants