Run pulpcore-manager check --deploy in acceptance#155
Run pulpcore-manager check --deploy in acceptance#155ekohl wants to merge 1 commit intotheforeman:masterfrom
Conversation
e553812 to
e8baccc
Compare
|
So there's 9 warnings:
We follow https://docs.pulpproject.org/pulpcore/installation/authentication.html#webserver-authentication and that causes this. Need to reach out to upstream.
I think HSTS is irrelevant for Pulp. Some content should be available over HTTPS and most API clients ignore HSTS anyway. It's mostly browsers. Should be masked. Maybe even from an upstream point of view?
Reading https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Content-Type-Options this looks like it might be a good thing to include. Should talk to upstream about this because I don't know how much security it actually brings.
Reading https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-XSS-Protection I wonder if this is really relevant. Especially considering Pulp is mostly an API anyway.
I think this should be masked since there is deliberate HTTP content.
Looks like we should really up our secret key generation.
Not sure if this is relevant since we don't use sessions. I think it can be masked.
Sounds like something upstream should do?
Also a good question to upstream. |
|
@daviddavis you asked me about checks. IMHO this is something the installer could run as well. sosreport is another. |
|
@ekohl, this pull request is currently not mergeable. Please rebase against the master branch and push again. If you have a remote called 'upstream' that points to this repository, you can do this by running: This message was auto-generated by Foreman's prprocessor |
e8baccc to
7ad6e8b
Compare
7ad6e8b to
a1b319b
Compare
Django has a checks framework that can detect problems in a deployment. It's also extensible and allows Pulp and plugin developers to add their own checks. This allows detection of misconfigurations. Some checks run implicitly before running certain commands but others don't for performance reasons. --deploy signals that a production setup is used, which enables more checks. https://docs.djangoproject.com/en/4.2/topics/checks/
a1b319b to
1177b97
Compare
|
I've dropped the |
|
|
||
| describe command('sudo -u pulp PULP_SETTINGS=/etc/pulp/settings.py pulpcore-manager check --deploy') do | ||
| its(:exit_status) { is_expected.to eq 0 } | ||
| its(:stderr) { is_expected.not_to match(/System check identified some issues:/) } |
There was a problem hiding this comment.
Does rspec validate all assertions, or does it stop at the first failing one? If it's the first, asserting the output first makes the result better actionable.
There was a problem hiding this comment.
Should be all as separate checks. It may also execute the command for every check so that's a bit slower.
|
I see a lot of type hint errors. @evgeni did you also see these in foremanctl? |
|
I think so, yes. But as those are only warnings, the exit code should still be 0. Maybe that's why we don't assert the stdout in foremanctl? |
Django has a checks framework that can detect problems in a deployment. It's also extensible and allows Pulp and plugin developers to add their own checks. This allows detection of misconfigurations.
Some checks run implicitly before running certain commands but others don't for performance reasons.
--deploy signals that a production setup is used, which enables more checks.
https://docs.djangoproject.com/en/2.2/topics/checks/