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

Read FQDNRejectResponseCode from config #27362

Merged

Conversation

ayuspin
Copy link
Contributor

@ayuspin ayuspin commented Aug 8, 2023

Currently dnsProxy.dnsRejectResponseCode helm value is ignored because FQDNRejectResponseCode is not populated from viper

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 8, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Aug 8, 2023
@ayuspin ayuspin force-pushed the pr/read-fqdnrejectresponsecode-from-config branch from 2f5ac26 to 9f60b6b Compare August 8, 2023 19:56
@ayuspin ayuspin marked this pull request as ready for review August 8, 2023 20:04
@ayuspin ayuspin requested review from a team as code owners August 8, 2023 20:04
@ayuspin ayuspin force-pushed the pr/read-fqdnrejectresponsecode-from-config branch from 9f60b6b to f143f0b Compare August 8, 2023 20:15
Copy link
Contributor

@thorn3r thorn3r left a comment

Choose a reason for hiding this comment

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

Nice find 👍

@ti-mo ti-mo added release-note/bug This PR fixes an issue in a previous release of Cilium. sig/agent Cilium agent related. labels Aug 16, 2023
@ti-mo
Copy link
Contributor

ti-mo commented Aug 16, 2023

/test

@ti-mo
Copy link
Contributor

ti-mo commented Aug 16, 2023

@ayuspin You'll need to rebase to pick up the Go version bump needed for CI to pass.

@ti-mo ti-mo removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 16, 2023
@ayuspin ayuspin force-pushed the pr/read-fqdnrejectresponsecode-from-config branch from f143f0b to 349b326 Compare August 16, 2023 15:48
@ayuspin
Copy link
Contributor Author

ayuspin commented Aug 16, 2023

@ti-mo done

@joestringer
Copy link
Member

It looks like you unfortunately picked a commit to base on main where the branch was broken. I'll click the rebase button to get this unblocked.

Currently dnsProxy.dnsRejectResponseCode helm value is ignored because FQDNRejectResponseCode is not populated from viper

Signed-off-by: Andrii Iuspin <yuspin@gmail.com>
@joestringer joestringer force-pushed the pr/read-fqdnrejectresponsecode-from-config branch from 349b326 to 8084fb1 Compare August 24, 2023 04:20
@joestringer
Copy link
Member

/test

@ti-mo ti-mo merged commit 87518a3 into cilium:main Aug 24, 2023
60 checks passed
@christarazi christarazi added area/fqdn Affects the FQDN policies feature affects/v1.12 This issue affects v1.12 branch affects/v1.13 This issue affects v1.13 branch affects/v1.14 This issue affects v1.14 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 24, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.2 Aug 24, 2023
@pippolo84 pippolo84 mentioned this pull request Aug 28, 2023
9 tasks
@pippolo84 pippolo84 added the backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. label Aug 28, 2023
@pippolo84 pippolo84 removed the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Aug 28, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.2 Aug 28, 2023
pippolo84 added a commit to pippolo84/cilium that referenced this pull request Aug 29, 2023
Commit 87518a3 ("Read FQDNRejectResponseCode from config") fixed the
FQDNRejectResponseCode option handling, adding the missing code to read
the value from the ConfigMap.  Since the usage of StringVar does not
correctly handle that case by itself, using it is pointless.

This commit changes it to use String as we already do for every other
string agent option.

Relates: cilium#27362

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
tklauser pushed a commit that referenced this pull request Aug 30, 2023
Commit 87518a3 ("Read FQDNRejectResponseCode from config") fixed the
FQDNRejectResponseCode option handling, adding the missing code to read
the value from the ConfigMap.  Since the usage of StringVar does not
correctly handle that case by itself, using it is pointless.

This commit changes it to use String as we already do for every other
string agent option.

Relates: #27362

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
@michi-covalent michi-covalent added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Sep 9, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.2 Sep 9, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.2 Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/v1.12 This issue affects v1.12 branch affects/v1.13 This issue affects v1.13 branch affects/v1.14 This issue affects v1.14 branch area/fqdn Affects the FQDN policies feature backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. kind/community-contribution This was a contribution made by a community member. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/agent Cilium agent related.
Projects
No open projects
1.14.2
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

None yet

7 participants