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.

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

Don't copy inexistent certs folder in Dockerfile.django #2946

Merged
merged 3 commits into from
Oct 11, 2020
Merged

Don't copy inexistent certs folder in Dockerfile.django #2946

merged 3 commits into from
Oct 11, 2020

Conversation

uncycler
Copy link
Contributor

@uncycler uncycler commented Oct 1, 2020

When building Dockerfile.django with DOCKER_BUILDKIT=1 option in docker, the build fails with:

failed to solve with frontend dockerfile.v0: failed to build LLB: lstat /var/lib/docker/tmp/buildkit-mount548069851/certs: no such file or directory

  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.6 compliant (specific python >3.6 syntax is currently not accepted).
  • If this is a new feature and not a bug fix, you've included the proper documentation in the ReadTheDocs documentation folder. https://github.com/DefectDojo/Documentation/tree/master/docs or provide feature documentation in the PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

@dsever
Copy link
Contributor

dsever commented Oct 2, 2020

@uncycler I added this option intentionally, see https://github.com/DefectDojo/Documentation/blob/master/docs/social-authentication.rst#saml-20

Idea is to enable generic way of inserting of private CA certs that are not included inside of certifi, problem is that I couldn't fine nice way to do optional copy e.g https://forums.docker.com/t/copy-only-if-file-exist/3781/2 . If you have better proposal please share it here.

@uncycler
Copy link
Contributor Author

uncycler commented Oct 2, 2020

@dsever Maybe just adding a certs directory with a readme.txt. We could put it inside /docker so it don't clobber the root. What do you think?

@dsever
Copy link
Contributor

dsever commented Oct 2, 2020

You mean just to have some content, not to break the build? If yes than I can add readme.txt and some instructions to it.

so /docker/certs/readme.txt?

For sure we need have DOCKER_BUILDKIT=1 option when we are building images

@uncycler
Copy link
Contributor Author

uncycler commented Oct 2, 2020

I update the PR to add a placeholder.

Copy link
Collaborator

@madchap madchap left a comment

Choose a reason for hiding this comment

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

lgtm.

Copy link
Member

@valentijnscholten valentijnscholten left a comment

Choose a reason for hiding this comment

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

ah great, I was thinking about a placeholder also but didn't see the PR was already updated with that. approving..

@valentijnscholten valentijnscholten merged commit 76ff674 into DefectDojo:dev Oct 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants