Skip to content

Conversation

Programmer-Admin
Copy link
Contributor

@Programmer-Admin Programmer-Admin commented Aug 20, 2025

Changes Incorporated:

  1. Upgraded haproxy version to 3.2.4 from 2.2
  2. Since haproxy now runs as haproxy user line #102, I placed haproxy.cfg file, inside /tmp. That makes no security tampering from USER side from inside of our Dockerfile.
  3. Changed the docker-socket-proxy version from 0.0.0 to 1.0.0, since we upgraded the haproxy to a major version.

Happy to contribute.

@pedrobaeza
Copy link
Member

Are you aware of the problems depicted here: #149 (comment) ?

@Programmer-Admin
Copy link
Contributor Author

@pedrobaeza - Yes, I am aware of #149 , that is why I am not starting the container as root, also no docker group addition is required in this setup. I tested this setup on multiple open source services, it is working as expected. Please let me know, if you feel otherwise. This setup does not requires any group addition in docker as was discussed in #149 (comment)

@pedrobaeza
Copy link
Member

OK, great, but please check CI.

@Programmer-Admin
Copy link
Contributor Author

I'm stuck in fixing the CI 😞. It would be of great help if someone could help me on that.

@josep-tecnativa
Copy link
Contributor

First of all, you could run pre-commit run -a to pass pre-commit stage.

@Programmer-Admin
Copy link
Contributor Author

@josep-tecnativa - I ran, then I see all files find themselves in Git Staging Area, where I was expecting only haproxy.cfg and Dockerfile to only be in staging area. Kinda confused on this behavior.

@Programmer-Admin
Copy link
Contributor Author

@pedrobaeza - Ok, finally I fixed CI part. Requesting you to please check.
Thanks @josep-tecnativa for helping me out!

@Programmer-Admin
Copy link
Contributor Author

Hello @josep-tecnativa, @pedrobaeza,
Ok, I see what's happening here, I actually used Docker Socket Proxy Over Nginx Proxy(as TLS) which listens on 2376 and forward the request to underlying docker-socket-proxy container. But, what I was not aware that the tests will be running with only this container being executed standalone, and then checking a couple of tests. Sorry for bothering you. Can you please check whether my approach works. Thanks!

@Programmer-Admin
Copy link
Contributor Author

@josep-tecnativa / @pedrobaeza - Could you please approve the workflow, so that I can check if something went wrong down the line. Thanks!

@pedrobaeza
Copy link
Member

Done, @Programmer-Admin

@Programmer-Admin
Copy link
Contributor Author

Many thanks @pedrobaeza. Seems like all checks passed. Now over to you for approval.

@pedrobaeza
Copy link
Member

OK, my colleague @josep-tecnativa will check, but at least the commits should be squashed into one.

@josep-tecnativa
Copy link
Contributor

Looks good to me!

Just squash your commits and we will merge.

http-request deny
default_backend dockerbackend

default_backend dockerbackend
Copy link
Member

Choose a reason for hiding this comment

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

Restore the line as it was for not having unneeded diff, please.

Copy link
Member

Choose a reason for hiding this comment

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

This is still pending.

Copy link
Contributor Author

@Programmer-Admin Programmer-Admin Aug 22, 2025

Choose a reason for hiding this comment

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

@pedrobaeza - I was actually thinking of keeping the use_backend and default_backend in separate section. Can we have that like this way? Or you want me to remove the new line before default_backend

Copy link
Member

Choose a reason for hiding this comment

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

They are not different sections in fact. I prefer to keep the diff minimal and don't add noise in any change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pedrobaeza - Could you please check now?

@pedrobaeza
Copy link
Member

Please squash together all the commits.

@Programmer-Admin
Copy link
Contributor Author

Please squash together all the commits.

I am unfamiliar with the squashing command. Can you please help me out? What command to issue for achieving squash.

@josep-tecnativa
Copy link
Contributor

Please squash together all the commits.

I am unfamiliar with the squashing command. Can you please help me out? What command to issue for achieving squash.

You can squash the commits using an interactive rebase. For example:

git fetch origin
git rebase -i origin/master

In the editor that opens, leave the first commit as pick and change all the others to f (for fixup) or s (for squash). Then save and close the editor.

This will combine all the commits into a single one on top of master. Finally,you will need to push with force:

git push --force

@Programmer-Admin
Copy link
Contributor Author

Thanks for the help @josep-tecnativa. Commits successfully squashed.

@pedrobaeza

This comment was marked as resolved.

@Programmer-Admin

This comment was marked as resolved.

Fixed CI

Reverted conftests.py, Added root user before CMD in Dockerfile, to respect existing setups

version pinned and comment removed from cfg

Removed extra empty lines

Addressed extra newline in haproxy.cfg
@niklasteichmann
Copy link

Hi,
The solution you implemented is giving the user in the image root permissions. I'm not sure if that's a good idea - but it's not worse than it was before the upgrade, so maybe that's okay for now. I'd just like to hear your thoughts about this.

@Programmer-Admin
Copy link
Contributor Author

@niklasteichmann - you are correct, it's not a good practice of giving Root user access inside docker image. But, if you see the original docker socket proxy image, even though we are not defining root in Dockerfile in the earlier version, if you exec, you will see that the earlier image is also using root user, that root user is actually propagating to socket proxy image from haproxy:2.2-alpine. But, now haproxy user is getting inherited from haproxy image to our socket proxy image, due to which I am manually changing the user to Root, so that the migration will be seamless and smooth.

@Programmer-Admin
Copy link
Contributor Author

@josep-tecnativa - can we please check on the merge status? Many Thanks

@niklasteichmann
Copy link

@Programmer-Admin Yeah, I understand that. It's the same level of security as before the base image upgrade. Maybe this can be resolved in a future PR, I'll keep using my fork for now.
Thanks for your work on this.

@josep-tecnativa
Copy link
Contributor

Sorry for the delay @Programmer-Admin , i was on holidays.

Looks good to me, LGTM!

@josep-tecnativa josep-tecnativa merged commit 0919fd7 into Tecnativa:master Aug 28, 2025
3 checks passed
alexlebens pushed a commit to alexlebens/infrastructure that referenced this pull request Aug 28, 2025
)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [ghcr.io/tecnativa/docker-socket-proxy](https://github.com/Tecnativa/docker-socket-proxy) | minor | `0.3.0` -> `v0.4.0` |

---

### Release Notes

<details>
<summary>Tecnativa/docker-socket-proxy (ghcr.io/tecnativa/docker-socket-proxy)</summary>

### [`v0.4.0`](https://github.com/Tecnativa/docker-socket-proxy/releases/tag/v0.4.0)

[Compare Source](Tecnativa/docker-socket-proxy@v0.3.0...v0.4.0)

#### What's Changed

- test: check if connection upgrades work (using exec) by [@&#8203;proudier](https://github.com/proudier) in Tecnativa/docker-socket-proxy#139
- \[ADD] v1.51 version to README.md and Modernice CI by [@&#8203;josep-tecnativa](https://github.com/josep-tecnativa) in Tecnativa/docker-socket-proxy#152
- Updated the underlying haproxy version from 2.2 to 3.2.4 by [@&#8203;Programmer-Admin](https://github.com/Programmer-Admin) in Tecnativa/docker-socket-proxy#156

#### New Contributors

- [@&#8203;Programmer-Admin](https://github.com/Programmer-Admin) made their first contribution in Tecnativa/docker-socket-proxy#156

**Full Changelog**: Tecnativa/docker-socket-proxy@v0.3.0...v0.4.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4zNS4wIiwidXBkYXRlZEluVmVyIjoiNDEuMzUuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiaW1hZ2UiXX0=-->

Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/1326
Co-authored-by: Renovate Bot <[email protected]>
Co-committed-by: Renovate Bot <[email protected]>
@Programmer-Admin Programmer-Admin deleted the dsp-haproxy-3.2.4 branch September 3, 2025 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants