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

Update crs 3.4 dev #12

Closed
wants to merge 3 commits into from
Closed

Conversation

fzipi
Copy link
Member

@fzipi fzipi commented Jul 9, 2020

This PR will:

  • update repo link on v3.3, and point it to master
  • add the new v3.4/dev

fzipi added 3 commits July 9, 2020 14:15
Signed-off-by: Felipe Zipitria <[email protected]>
Signed-off-by: Felipe Zipitria <[email protected]>
Signed-off-by: Felipe Zipitria <[email protected]>
@bittner
Copy link
Contributor

bittner commented Aug 2, 2020

Good that we didn't merge this yet.

The original idea was that we have a single source of truth for the Dockerfile and its related configuration, which should ideally live in the src folder, and there should be a Makefile (or something similar) to (re)generate the Dockerfiles for all the currently supported CRS versions and web proxy combinations.

This was originally described in coreruleset/modsecurity-docker#32 and related issues. Looks like we missed out on implementing this to the full extend, as originally planned.

What Next?

We could

  • either merge this PR "as is", and attempt the scripted implementation of generating all Dockerfile setups as a follow-up step
  • or you could try to implement the scripted approach with this PR directly, before merging it into master.

Opinions?

(Personally, I'd prefer to add another commit with the scripted generation to this PR, already. This would make sure the job gets done more likely, I believe. Just a personal opinion, though.)

@bittner
Copy link
Contributor

bittner commented Aug 7, 2020

@fzipi Do you want to give the generalization a try?

I'd suggest we do this like in our appuio/container-oc repository. There we have a src folder and version folders that are generated from src with a shell script that mainly does some sed magic. The Makefile in the root folder is for convenience.

@fzipi
Copy link
Member Author

fzipi commented Aug 7, 2020

@bittner Sure. Do you think we should add it here, or create a new PR and close this one?

@bittner
Copy link
Contributor

bittner commented Aug 10, 2020

Do you think we should add it here, or create a new PR and close this one?

I think it's fine to replace the commits in this PR. But if you want to keep this PR for future reference go forward, close it and start a new PR. Fine with me.

@fzipi
Copy link
Member Author

fzipi commented Aug 30, 2020

Need to make the change here by replacing the PR.

@fzipi
Copy link
Member Author

fzipi commented Mar 7, 2021

Fixed in #25. Closing this one.

@fzipi fzipi closed this Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants