feat: add support for passing resolv.conf in dns discovery by Revolyssup · Pull Request #9770 · apache/apisix

Revolyssup · 2023-07-03T05:39:37Z

Description

Fixes #9565

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

Signed-off-by: revolyssup <[email protected]>

Revolyssup · 2023-08-09T07:28:00Z

@monkeyDluffy6017 Is this feature desirable?

juststillthinking · 2023-08-14T07:39:38Z

--- grep_error_log eval
-qr/connect to 127.0.0.1:1053/
--- grep_error_log_out
-connect to 127.0.0.1:1053
-connect to 127.0.0.1:1053
-connect to 127.0.0.1:1053
-connect to 127.0.0.1:1053
-connect to 127.0.0.1:1053


Why do you delete this?

Must have been done by mistake while testing something. Pushed the latest commit

Signed-off-by: revolyssup <[email protected]>

juststillthinking · 2023-08-14T08:02:10Z

                type = "string",
            },
        },
+        resolvconf = {


We should add this to config-default.yaml

config-default already has servers

apisix/conf/config-default.yaml

Line 279 in 01763b5

# servers:

and only one of servers or resolvconf is needed. Should I add it anyways?

You should add the resolv_conf to the config-default.yaml, or nobody will find this attribute, add comments to clarify that servers or resolvconf is mutual exclusive

Okay sounds good. Adding it..

juststillthinking · 2023-08-14T08:07:19Z

    local opts = {
        hosts = {},
-        resolvConf = {},
+        resolvConf = resolvconf,


Use resolv_conf to replace all variables

okay except for the resolveConf key inside opts because that field is in the lua-resty-dns.

Signed-off-by: revolyssup <[email protected]>

juststillthinking · 2023-08-14T08:27:00Z

+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+nameserver 127.0.0.1:1053


Maybe you could create the file in build-cache dynamically like the function set_coredns

Signed-off-by: Ashish Tiwari <[email protected]>

juststillthinking · 2023-08-16T16:25:11Z

Please make the ci pass

Signed-off-by: Ashish Tiwari <[email protected]>

Revolyssup · 2023-09-25T09:32:37Z

@monkeyDluffy6017 Tests passing now

juststillthinking · 2023-09-25T09:55:09Z

    ../../build-cache/coredns -dns.port=1053 &
    popd || exit 1
+
+


Remove redundant blank

juststillthinking · 2023-09-25T09:55:50Z

 connect to 127.0.0.1:1053
 connect to 127.0.0.1:1053
 connect to 127.0.0.1:1053
+connect to 127.0.0.1:1053


Why do you add this? it doesn't look like it has anything to do with your modifications

earlier it was trying 4 times but when resolv.conf is used it tries one more times(5 times)

Signed-off-by: Ashish Tiwari <[email protected]>

Revolyssup added 5 commits July 3, 2023 11:08

feat: add support for passing resolv.conf in dns discovery

0f5e35f

Signed-off-by: revolyssup <[email protected]>

fix lint issue

71f279f

Signed-off-by: revolyssup <[email protected]>

fix license

0b40ed7

Signed-off-by: revolyssup <[email protected]>

fix lint issue

0a3ef39

Signed-off-by: revolyssup <[email protected]>

Merge remote-tracking branch 'upstream/master' into revolyssup/fix9565

63155d3

juststillthinking added the discuss label Jul 18, 2023

Revolyssup mentioned this pull request Jul 18, 2023

help request: in discovery.dns.servers config, how to use local DNS resolver ? #9565

Closed

Revolyssup added the feature-request label Aug 9, 2023

Revolyssup requested a review from juststillthinking August 9, 2023 07:27

juststillthinking reviewed Aug 14, 2023

View reviewed changes

readd assertions on test

a680a95

Signed-off-by: revolyssup <[email protected]>

juststillthinking reviewed Aug 14, 2023

View reviewed changes

replace resolvconf with resolv_conf

e3b3205

Signed-off-by: revolyssup <[email protected]>

juststillthinking reviewed Aug 14, 2023

View reviewed changes

juststillthinking added wait for update wait for the author's response in this issue/PR and removed discuss feature-request labels Aug 14, 2023

Revolyssup added 3 commits August 16, 2023 11:05

apply suggestions

58a89d1

Signed-off-by: Ashish Tiwari <[email protected]>

Merge remote-tracking branch 'upstream/master' into revolyssup/fix9565

df7a645

fix lint test

3cab090

Signed-off-by: Ashish Tiwari <[email protected]>

Revolyssup added 3 commits August 17, 2023 10:43

rearrange command in script

e6146d2

Merge remote-tracking branch 'upstream/master' into revolyssup/fix9565

b6e6816

add one more attempt log to mix.t

127491a

Signed-off-by: Ashish Tiwari <[email protected]>

github-actions Bot added the user responded label Sep 25, 2023

Revolyssup removed the wait for update wait for the author's response in this issue/PR label Sep 25, 2023

juststillthinking reviewed Sep 25, 2023

View reviewed changes

remove redundant line

f30e50d

Signed-off-by: Ashish Tiwari <[email protected]>

juststillthinking approved these changes Sep 25, 2023

View reviewed changes

juststillthinking added approved and removed user responded labels Sep 25, 2023

shreemaan-abhishek approved these changes Sep 26, 2023

View reviewed changes

juststillthinking merged commit ddac444 into apache:master Sep 26, 2023

kayx23 added a commit that referenced this pull request Oct 8, 2023

remove the duplicated entry PR #9770 for resolv_conf for local DNS

6e74219

Uh oh!

Conversation

Revolyssup commented Jul 3, 2023

Description

Checklist

Uh oh!

Revolyssup commented Aug 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juststillthinking Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Revolyssup Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juststillthinking Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juststillthinking commented Aug 16, 2023

Uh oh!

Revolyssup commented Sep 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juststillthinking Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Revolyssup Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

juststillthinking Aug 14, 2023 •

edited

Loading

Revolyssup Aug 14, 2023 •

edited

Loading

juststillthinking Aug 14, 2023 •

edited

Loading

juststillthinking Sep 25, 2023 •

edited

Loading

Revolyssup Sep 25, 2023 •

edited

Loading