Project

Profile

Help

Issue #2230

sync operation accepts invalid upstream_name

Added by jluza over 5 years ago. Updated over 2 years ago.

Status:
CLOSED - WONTFIX
Priority:
Normal
Assignee:
-
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version - Docker:
2.1.0
Platform Release:
Target Release - Docker:
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Pulp 2
Sprint:
Quarter:

Description

I can trigger following sync task:

curl -X POST https://pulp-dev:443/pulp/api/v2/repositories/1a18d18e-b20e-471d-baa2-6aa108941122/actions/sync/ --data '{"override_config": {"feed": "http://crane01.web.qa.ext.phx1.redhat.com", "upstream_name": "give me some space", "ssl_validation": false, "enable_v1": true}}'

And task fails with

Traceback (most recent call last):
 File "/usr/lib/python2.7/site-packages/celery/app/trace.py", line 240, in trace_task
   R = retval = fun(*args, **kwargs)
 File "/home/vagrant/devel/pulp/server/pulp/server/async/tasks.py", line 488, in __call__
   return super(Task, self).__call__(*args, **kwargs)
 File "/home/vagrant/devel/pulp/server/pulp/server/async/tasks.py", line 103, in __call__
   return super(PulpTask, self).__call__(*args, **kwargs)
 File "/usr/lib/python2.7/site-packages/celery/app/trace.py", line 438, in __protected_call__
   return self.run(*args, **kwargs)
 File "/home/vagrant/devel/pulp/server/pulp/server/controllers/repository.py", line 760, in sync
   sync_report = sync_repo(transfer_repo, conduit, call_config)
 File "/home/vagrant/devel/pulp/server/pulp/server/async/tasks.py", line 673, in wrap_f
   return f(*args, **kwargs)
 File "/home/vagrant/devel/pulp_docker/plugins/pulp_docker/plugins/importers/importer.py", line 84, in sync_repo
   return self.sync_step.process_lifecycle()
 File "/home/vagrant/devel/pulp/server/pulp/plugins/util/publish_step.py", line 565, in process_lifecycle
   super(PluginStep, self).process_lifecycle()
 File "/home/vagrant/devel/pulp/server/pulp/plugins/util/publish_step.py", line 162, in process_lifecycle
   step.process()
 File "/home/vagrant/devel/pulp/server/pulp/plugins/util/publish_step.py", line 252, in process
   self._process_block()
 File "/home/vagrant/devel/pulp/server/pulp/plugins/util/publish_step.py", line 296, in _process_block
   self.process_main()
 File "/home/vagrant/devel/pulp_docker/plugins/pulp_docker/plugins/importers/sync.py", line 223, in process_main
   available_tags = self.parent.index_repository.get_tags()
 File "/home/vagrant/devel/pulp_docker/plugins/pulp_docker/plugins/registry.py", line 406, in get_tags
   reason=str(e))
PulpCodedException: Could not fetch repository give me some space from registry http://crane01.web.qa.ext.phx1.redhat.com - NOT FOUND

Since there obviously can't be space in upstream_name and also some restriction are applied on image name:
https://github.com/docker/docker/issues/313
I suppose, pulp should at least fail with different error message

History

#1 Updated by amacdona@redhat.com over 5 years ago

  • Triaged changed from No to Yes

#2 Updated by ttereshc over 5 years ago

  • Platform Release deleted (2.10.1)
  • Target Release - Docker deleted (2.1.0)

#3 Updated by Ichimonji10 about 5 years ago

jluza and devs: What criteria must be met for this bug to be fixed? How about requiring that an HTTP 400 response be returned to clients when an HTTP POST request is made to /pulp/api/v2/repositories/{repository_id}/actions/sync/ containing an invalid upstream_name? This contrasts with the current behaviour of returning an HTTP 202 and only failing after the task has started.

This bug report references https://github.com/docker/docker/issues/313. However, I do not see any reference to Docker repository naming rules therein. (I didn't read the entire thread in full, so I may have missed something.) Perhaps the HTTP API V2 page in the official Docker docs will be of more use in fixing this bug. Importantly, it states the following:

Classically, repository names have always been two path components where each path component is less than 30 characters. The V2 registry API does not enforce this. The rules for a repository name are as follows:

1. A repository name is broken up into path components. A component of a repository name must be at least one lowercase, alpha-numeric characters, optionally separated by periods, dashes or underscores. More strictly, it must match the regular expression [a-z0-9]+(?:[._-][a-z0-9]+)*.
2. If a repository name has two or more path components, they must be separated by a forward slash (“/”).
3. The total length of a repository name, including slashes, must be less the 256 characters.

These name requirements only apply to the registry API and should accept a superset of what is supported by other docker ecosystem components.

#4 Updated by jluza about 5 years ago

I should really buy glasses

1. A repository name is broken up into path components. A component of a repository name must be at least one lowercase, alpha-numeric characters, optionally separated by periods, dashes or underscores. More strictly, it must match the regular expression [a-z0-9]+(?:[._-][a-z0-9]+)*.

Yes, that's what I was looking for in v2 docs.
400 sounds good, it's much better then NOT FOUND which imply url is correct, but simply returns 404.
If you for example choose registry-id: " foo/bar", you will end with

PulpCodedException: Could not fetch repository  foo/bar from registry http://crane01.web.qa.ext.phx1.redhat.com - NOT FOUND

you can easily overlook extra space. 400 with details about unmatched registry-id regexp is much more explaining in my opinion.

#5 Updated by bmbouter over 4 years ago

  • Tags RCM added

#6 Updated by bmbouter over 2 years ago

  • Status changed from NEW to CLOSED - WONTFIX

#7 Updated by bmbouter over 2 years ago

Pulp 2 is approaching maintenance mode, and this Pulp 2 ticket is not being actively worked on. As such, it is being closed as WONTFIX. Pulp 2 is still accepting contributions though, so if you want to contribute a fix for this ticket, please reopen or comment on it. If you don't have permissions to reopen this ticket, or you want to discuss an issue, please reach out via the developer mailing list.

#8 Updated by bmbouter over 2 years ago

  • Tags Pulp 2 added

#9 Updated by bmbouter over 2 years ago

  • Tags deleted (RCM)

Please register to edit this issue

Also available in: Atom PDF