-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Resolves issue #308 #386
Resolves issue #308 #386
Conversation
Talk to @seizadi, he is trying to add Kubernetes support which needs Docker. I suggested using this https://github.com/opsxcq/docker-vulnerable-dvwa but found out it isn't supported any more so we would probably be better using a local version. I don't understand either technology and don't have the time to learn them so anything that is merged in will have to be supported by you or @seizadi. |
One thing that was nice in this commit was the use of Git Hub Actions to build the container and push it. If you are ok with this I can make this change to my PR or peter can submit an update when my PR is merged. This way every time there is an update to the project you have a new container. |
Is that the recommended way to do things? If so, do it.
…On Mon, 21 Sep 2020 at 08:58, Soheil Eizadi ***@***.***> wrote:
One thing that was nice in this commit was the use of Git Hub Actions to
build the container and push it. If you are ok with this I can make this
change to my PR or peter can submit an update when this PR is merged. This
way every time there is an update to the project you have a new container.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA4SWPB74SBG2QXBVIMM3DSG4BRHANCNFSM4RUCN2WQ>
.
|
I think using the K8s and Docker container are different approaches/purposes to build this vulnerable web application repository. And I think it's good to have these above approaches on this repository :). |
@seizadi, I think you can update your PR. After your PR has been merged and updated, I will update my PR :). Thanks. |
Do we need both PRs?
…On Mon, 21 Sep 2020 at 09:24, Chun-Sheng, Li ***@***.***> wrote:
One thing that was nice in this commit was the use of Git Hub Actions to
build the container and push it. If you are ok with this I can make this
change to my PR or peter can submit an update when my PR is merged. This
way every time there is an update to the project you have a new container.
@seizadi <https://github.com/seizadi>, I think you can update your PR.
After your PR has been merged and updated, I will update my PR :). Thanks.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA4SWPZ5YFD4C6SVWSDTS3SG4ET7ANCNFSM4RUCN2WQ>
.
|
I think we need both because their purposes are different. The K8s support is for the Cyber-Defense system and it needs a robust machine and the Docker container setup is for developers and hackers on their local development environment. |
OK, as long as they stay in sync and both do the jobs they are designed to
do.
On Mon, 21 Sep 2020 at 09:36, Chun-Sheng, Li <notifications@github.com>
wrote:
… Do we need both PRs?
… <#m_-7267607401811733145_>
On Mon, 21 Sep 2020 at 09:24, Chun-Sheng, Li *@*.***> wrote: One thing
that was nice in this commit was the use of Git Hub Actions to build the
container and push it. If you are ok with this I can make this change to my
PR or peter can submit an update when my PR is merged. This way every time
there is an update to the project you have a new container. @seizadi
<https://github.com/seizadi> https://github.com/seizadi, I think you can
update your PR. After your PR has been merged and updated, I will update my
PR :). Thanks. — You are receiving this because you commented. Reply to
this email directly, view it on GitHub <#386 (comment)
<#386 (comment)>>, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAA4SWPZ5YFD4C6SVWSDTS3SG4ET7ANCNFSM4RUCN2WQ
.
I think we need both because their purposes are different.
The K8s support is for the CDX system and robust machine and the Docker
container setup is for developers and hackers on their local development
environment.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA4SWPD6MDHF53LEJTORN3SG4GBBANCNFSM4RUCN2WQ>
.
|
@digininja, @seizadi. I figure out another approach about resolving the issue. I create a repository named peter279k/docker-dvwa and build the Docker image with the latest commit on this repository. Then publishing the Docker image to the Dockernub page. And I also replace the Dockerhub page link on this section. I think it's fine to let the PR #384 be merged because I think it's fine to have the K8s support officially for this repository. And it's more important/powerful than building the Docker container setup. What do you think about that? |
Is that going to give a continuously up to date image or a static one based
on what is there today?
…On Mon, 21 Sep 2020 at 14:20, Chun-Sheng, Li ***@***.***> wrote:
@digininja <https://github.com/digininja>, @seizadi
<https://github.com/seizadi>. I figure out another approach about
resolving the issue.
I create a repository named peter279k/docker-dvwa and build the Docker
image with the latest commit on this repository.
Then publishing the Docker image to the Dockernub page. And I also replace
the Dockerhub page link on this section
<https://github.com/digininja/DVWA#docker-container>.
I think it's fine to let the PR #384
<#384> continuously because I think
it's fine to have the K8s support officially for this repository.
What do you think about that?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA4SWMW7EDJUWLR64K5YLLSG5HJFANCNFSM4RUCN2WQ>
.
|
@digininja, I plan to let the tag releasing be same as this repository. But it doesn't release new tag version for the long time. I think it's fine to up-to-date image via And the Docker image tag can be the short commit hash that is same as commit hash om this repository. |
If you look back at the tag history, making releases isn't something the
project has been good at and so unless this can use the latest head, it
will end up behind and potentially missing out on updates.
…On Mon, 21 Sep 2020 at 14:57, Chun-Sheng, Li ***@***.***> wrote:
@digininja <https://github.com/digininja>, I plan to let the tag
releasing be same as this repository.
But it doesn't release new tag version for the long time.
I think it's fine to up-to-date image via git clone command once this
repository has the latest commit on the master branch.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA4SWN3SCSTV4UJL6LDX3TSG5LWHANCNFSM4RUCN2WQ>
.
|
We can consider building the latest commit on master and don't care about tag history and it will be up-to-date. |
That would be better
…On Mon, 21 Sep 2020 at 15:08, Chun-Sheng, Li ***@***.***> wrote:
We can consider building the latest commit on master and don't care about
tag history and it will be up-to-date.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA4SWMQ7HYWIX4HEC4AN3DSG5M57ANCNFSM4RUCN2WQ>
.
|
If we setup Github Action, we will need to tag the system and release zip, container tag and if necessary update helm chart. So people that want something that works can use the tags. The latest is not guaranteed to work, as enough time goes by Dockerfile will need changes due to breaking changes in master. |
Let's assume that tagging and releases won't happen as despite best
efforts, I know I'll forget.
I've got an action setup that checks for broken links every time I commit a
change, can't you do the same as that does?
…On Mon, 21 Sep 2020 at 17:10, Soheil Eizadi ***@***.***> wrote:
If we setup Github Action, we will need to tag the system and release zip,
container tag and if necessary update helm chart. So people that want
something that works can use the tags. The latest is not guaranteed to
work, as enough time goes by Dockerfile will need changes due to breaking
changes in master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA4SWJ5DW7ZVYWP544T4ETSG53GVANCNFSM4RUCN2WQ>
.
|
I think it's possible and I think I can monitor the commits changing on this repository. Once the upcoming pull requests and commits are merged on the master branch, I will update the commit on my docker-dvwa repository. |
Why would be setup a separate Docker repo, I thought Robin agreed that once we get the Dockerfile in the PR merged, we would setup a GitHub Action to build and push the latest container. Also we would tag the current master and create a Docker image/tag with that version that we know works. Going forward you have a tag version that we know works and the latest container image. If someone is willing to test and tag a version you can get a new tagged version, from what I have seen this is at best an annual event. |
The problem will be, if someone finds a bug, the first thing I'm going to
say to them is to use the latest version from Github, I'm not supporting
anything other than that. If the docker version is behind, and all the know
is how to use docker, then they won't be able to update to the latest
version. It needs to be setup in a way that the docker container always
contains the latest code. That may be that we build something every time
there is a commit, or the container pulls the latest code from the repo
when it is started.
…On Tue, 22 Sep 2020 at 06:54, Soheil Eizadi ***@***.***> wrote:
Why would be setup a separate Docker repo, I think Robin agreed that once
we get the Dockerfile in the PR merged, we would setup a GitHub Action to
build and push the latest container. Also we would tag the current master
and create a Docker image/tag with that version that we know works. Going
forward you have a tag version that we know works and the latest container
image. If someone is willing to test and tag a version you can get a new
tagged version.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA4SWPW5L7MCD7MMTP6A53SHA32LANCNFSM4RUCN2WQ>
.
|
I have done something comparable, as there was no current Image available on Dockerhub and then stumbled across this thread here. My approach works independently of tagging or not tagging and doesn't require any human interaction for updates. The image just rebuilds itself automatically every night against the latest master branch of DVWA and then pushes itself to Dockerhub. This is regardless of new code in DVWA or not. So at any given time, the image is at max 24 hours old/behind. You can find the repo and its automation pipelines here: https://github.com/cytopia/docker-dvwa and might get some additional inspiration from another angle of doing it. Let me know if you need any support or explanation of the pipelines. |
That sounds like a good way to do it |
@its0x08 what are you approving? There are a bunch of different options given here that all seem to be suggesting better ways to do it than this PR. |
Closes #308.