Issue #2230
closed
sync operation accepts invalid upstream_name
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
Updated by ttereshc over 7 years ago
- Platform Release deleted (
2.10.1) - Target Release - Docker deleted (
2.1.0)
Updated by Ichimonji10 over 7 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.
Updated by jluza over 7 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.
Updated by bmbouter about 5 years ago
- Status changed from NEW to CLOSED - WONTFIX
Updated by bmbouter about 5 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.
.