Skip to content

feat(sdp): Hive cmd for standalone dns proxy #39906

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 5, 2025
Merged

feat(sdp): Hive cmd for standalone dns proxy #39906

merged 2 commits into from
Aug 5, 2025
+255 −56

Conversation

vipul-21
Copy link
Contributor

@vipul-21 vipul-21 commented Jun 5, 2025 •
edited
Loading

This pull request introduces a new standalone DNS proxy feature to the Cilium project. Key changes include the addition of a new module for the standalone DNS proxy, updates to configuration management, and the creation of supporting files for the proxy's functionality and testing.

Standalone DNS Proxy Implementation:

  • Added a new module StandaloneDNSProxyCell in standalone-dns-proxy/cmd/root.go to provide the standalone DNS proxy functionality, including hooks for starting and stopping the proxy based on configuration.
  • Introduced the StandaloneDNSProxy struct and its associated methods (StartStandaloneDNSProxy and StopStandaloneDNSProxy) in standalone-dns-proxy/cmd/standalonednsproxy.go. These methods are placeholders for future implementation.
  • Created the entry point for the standalone DNS proxy in standalone-dns-proxy/main.go. This file initializes the proxy and executes the command.

Configuration and Code Refactoring:

  • Renamed defaultConfig to DefaultConfig in pkg/fqdn/service/cell.go to align with Go naming conventions for exported variables.

Testing:

  • Added a test case in standalone-dns-proxy/cmd/standalonednsproxy_test.go to verify that the standalone DNS proxy module can be populated without errors.

Code Ownership:

  • Updated the CODEOWNERS file to assign ownership of the standalone-dns-proxy/ directory to the @cilium/proxy and @cilium/fqdn teams.

Fixes: #30984
CFP: cilium/design-cfps#54

Added initial scaffolding for a standalone DNS proxy component in Cilium. This includes a new module to manage the proxy lifecycle, configuration updates, and basic test coverage. The proxy functionality is currently a placeholder and will be expanded in future releases.

@maintainer-s-little-helper
Copy link

Commit 59d304f does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jun 5, 2025
@github-actions github-actions bot added the sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. label Jun 5, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jun 5, 2025
@vipul-21 vipul-21 marked this pull request as ready for review June 5, 2025 06:18
@vipul-21 vipul-21 requested review from a team as code owners June 5, 2025 06:18
@vipul-21 vipul-21 requested a review from bimmlerd June 5, 2025 06:18
joamaki
joamaki approved these changes Jun 5, 2025
View reviewed changes
@vipul-21 vipul-21 requested review from a team as code owners June 9, 2025 22:04
@vipul-21 vipul-21 requested a review from asauber June 9, 2025 22:04
@vipul-21 vipul-21 requested a review from joamaki June 9, 2025 23:20
bimmlerd
bimmlerd requested changes Jun 17, 2025
View reviewed changes
Copy link
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

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

minor nit, otherwise LGTM

@vipul-21 vipul-21 requested a review from bimmlerd June 17, 2025 21:55
@vipul-21
Copy link
Contributor Author

@asauber Friendly ping for review ! Thanks !

@vipul-21
Copy link
Contributor Author

/test

@joestringer joestringer added the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Jun 20, 2025
@joestringer joestringer removed the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Jul 1, 2025
@vipul-21 vipul-21 requested review from a team as code owners July 31, 2025 14:57
@vipul-21 vipul-21 requested a review from borkmann July 31, 2025 14:57
@vipul-21
Copy link
Contributor Author

/test

@vipul-21
Copy link
Contributor Author

CI investigations:

@vipul-21
Copy link
Contributor Author

/ci-clustermesh

@vipul-21
Copy link
Contributor Author

/ci-multi-pool

@vipul-21
Copy link
Contributor Author

vipul-21 commented Jul 31, 2025 •
edited
Loading

Ci Investigations:
ci-multi-pool was failing due to "command terminated with exit code 28" on test case : "seq-client-egress-l7-extra-tls-headers/pod-to-world-with-extra-tls-intercept:https-to-k8s.io.-0". Did not seem to be the code path that should be affected by this PR changes, so retrying.
No issue was opened, so opened one: #40854

@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 31, 2025
@vipul-21
Copy link
Contributor Author

vipul-21 commented Aug 5, 2025

@cilium/security Friendly ping for review! Thanks !

joestringer
joestringer reviewed Aug 5, 2025
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Changes look good from a security perspective.

We generally expect the git history to be a bit cleaner - there's a few hunks in commits that are unrelated to the changes being made, and the last couple of commits look like they should be part of the original commits introducing the broken changes. Otherwise if we bisect, some specific tests / checks may fail if a bisect lands in the middle of this PR. Mind fixing those up?

@joestringer joestringer self-assigned this Aug 5, 2025
@joestringer joestringer removed the request for review from borkmann August 5, 2025 16:37
vipul-21 added 2 commits August 5, 2025 20:08
@vipul-21
feat(sdp): Hive cmd for standalone dns proxy
282da82 
- Introduces the standalone dns proxy module
- Adds the CODEOWNERS for the standalone dns proxy module

Signed-off-by: Vipul Singh <singhvipul@microsoft.com>
@vipul-21
Move some fqdn config from agent
af17d29 
Signed-off-by: Vipul Singh <singhvipul@microsoft.com>
@vipul-21 vipul-21 requested a review from joestringer August 5, 2025 20:15
joestringer
joestringer approved these changes Aug 5, 2025
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@joestringer joestringer enabled auto-merge August 5, 2025 20:20
@joestringer
Copy link
Member

/test

@vipul-21
Copy link
Contributor Author

vipul-21 commented Aug 5, 2025

ci-eks is failing . There is an issue which is closed by the bot: #37721.
There is comment(and a commit) that talks about the situation to be improved: #37721 (comment), but I don't think the issue is resolved altogether ? cc: @joestringer

@vipul-21
Copy link
Contributor Author

vipul-21 commented Aug 5, 2025

/ci-eks

@joestringer joestringer added this pull request to the merge queue Aug 5, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 5, 2025
Merged via the queue into cilium:main with commit 2d5a128 Aug 5, 2025
70 checks passed
@joestringer
Copy link
Member

@vipul-21 right, the comment there was talking about how ownership was determined for the test, not the test case itself. We haven't seen this recently as far as I know, perhaps it's just a relatively rare failure. I've reopened the issue for now.

@vipul-21 vipul-21 mentioned this pull request Aug 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joamaki joamaki joamaki approved these changes

@joestringer joestringer joestringer approved these changes

@asauber asauber asauber approved these changes

@bimmlerd bimmlerd bimmlerd approved these changes

@HadrienPatte HadrienPatte HadrienPatte approved these changes

Assignees

@joestringer joestringer

Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CFP: Introduce HA mode for DNS proxy
6 participants
@vipul-21 @joamaki @joestringer @asauber @bimmlerd @HadrienPatte