example(outline-cli): configure system DNS when `resolv.conf` does not exist #203

zeslava · 2024-03-23T12:56:16Z

It's ok if no original resolv.conf file found

jyyi1

Thanks for the contribution!

jyyi1

Thanks for the contribution.

But this logic might not work when there is resolv.head.conf, but not resolv.conf. Previously, only resolv.conf.outlinecli.backup will be created and only resolv.conf will be changed; but now both resolv.head.conf and resolv.conf will be changed.

We might need to add a special logic to setSystemDNSServer to handle the situation that neither resolv.head.conf nor resolv.conf exist in the system. In that case, we just need to create a new resolv.conf with Outline's setting, and when we restoreSystemDNSServer, we need to delete the newly created resolv.conf (because no backups are available).

jyyi1

Thanks a lot for the update! I believe we can even simplify the logic by introducing a global backups variable here. So we don't need to have any restoration callback functions, which is a little bit harder to understand.

jyyi1 · 2024-04-29T15:41:11Z

x/examples/outline-cli/dns_linux.go

@@ -27,30 +28,58 @@ const (
 	resolvConfHeadBackupFile = "/etc/resolv.head.outlinecli.backup"
 )

-func setSystemDNSServer(serverHost string) error {
+// restoreBackup restore backup file function
+type restoreBackup func()


This might not be needed (see comments below). In stead, we can have a global variable storing the updated resolv file paths:

Suggested change

type restoreBackup func()

type resolvBackup struct {

original, backup string

}

var backups []resolvBackup

jyyi1 · 2024-04-29T15:55:34Z

x/examples/outline-cli/dns_linux.go

+	restoreOriginal, err := backupAndWriteFile(resolvConfFile, resolvConfBackupFile, setting)
+	restores = append(restores, restoreOriginal)
+	if err != nil {
+		return restore, err
+	}
+
+	restoreOriginalHead, err := backupAndWriteFile(resolvConfHeadFile, resolvConfHeadBackupFile, setting)
+	restores = append(restores, restoreOriginalHead)
+
+	return restore, err


I think we only need to handle the os.ErrNotExist condition in backupAndWriteFile. So we don't need to change a lot of things in this function.

jyyi1 · 2024-04-29T15:56:16Z

x/examples/outline-cli/dns_linux.go

+	restore := func() {
+		// by default restore original file from backup
+		restoreFileIfExists(backup, original)
+	}


This is not needed, we can add file paths to the backups variable.

Suggested change

restore := func() {

// by default restore original file from backup

restoreFileIfExists(backup, original)

}

x/examples/outline-cli/dns_linux.go

jyyi1 · 2024-04-29T15:59:24Z

x/examples/outline-cli/dns_linux.go

+		if errors.Is(err, os.ErrNotExist) {
+			// original file does not exist, so just remove created file by ourselves
+			restore = func() {
+				if err := os.Remove(original); err != nil {
+					logging.Warn.Printf("failed to remove our file '%s': %v\n", original, err)
+				}
+			}
+		} else {
+			return func() {}, fmt.Errorf("failed to backup DNS config file '%s' to '%s': %w", original, backup, err)
+		}


We just need to ignore os.ErrNotExist here. The paths will be appended to backups later.

Suggested change

if errors.Is(err, os.ErrNotExist) {

// original file does not exist, so just remove created file by ourselves

restore = func() {

if err := os.Remove(original); err != nil {

logging.Warn.Printf("failed to remove our file '%s': %v\n", original, err)

}

}

} else {

return func() {}, fmt.Errorf("failed to backup DNS config file '%s' to '%s': %w", original, backup, err)

}

if !errors.Is(err, os.ErrNotExist) {

return func() {}, fmt.Errorf("failed to backup DNS config file '%s' to '%s': %w", original, backup, err)

}

jyyi1 · 2024-04-29T15:59:45Z

x/examples/outline-cli/dns_linux.go

 }

-func backupAndWriteFile(original, backup string, data []byte) error {
+func backupAndWriteFile(original, backup string, data []byte) (restoreBackup, error) {


We don't need to return any restoration functions here.

Suggested change

func backupAndWriteFile(original, backup string, data []byte) (restoreBackup, error) {

func backupAndWriteFile(original, backup string, data []byte) error {

jyyi1 · 2024-04-29T16:06:30Z

x/examples/outline-cli/dns_linux.go

 	}
-	return nil
+
+	return restore, err
 }

 func restoreFileIfExists(backup, original string) {


And finally, you may simply loop though all entries in backups here (we don't need to take any backup or original parameters in this function any more):

// remember to remove restored entries from `backups` after this function is successfully run for _, backup := range backups { if _, err := os.Stat(backup.backup); err != nil { // if err is notexist // simplify remove file backup.original // logging.Info.Printf("DNS config '%s' removed\n", backup.original) // continue logging.Warn.Printf("filed to get info of DNS config backup file '%s': %v\n", backup, err) return } if err := os.Rename(backup.backup, backup.original); err != nil { logging.Err.Printf("failed to restore DNS config from backup '%s' to '%s': %v\n", backup.backup, backup.original, err) return } logging.Info.Printf("DNS config restored from '%s' to '%s'\n", backup.backup, backup.original) }

zeslava · 2024-04-30T20:12:27Z

@jyyi1 fixed

jyyi1

The code looks good to me, with a few minor comments. Please also exclude the unrelated style changes in this PR. Appreciated!

jyyi1 · 2024-05-01T20:19:53Z

transport/tlsfrag/tls.go

-type tlsHandshakeRecordHeader []byte
+// This file contains helper functions and constants for TLS Client Hello message.
+
+type (


Please revert these changes, they are not related to this PR.

jyyi1 · 2024-05-01T20:21:59Z

x/examples/outline-cli/dns_linux.go

@@ -27,40 +28,77 @@ const (
 	resolvConfHeadBackupFile = "/etc/resolv.head.outlinecli.backup"
 )

+// systemDNSBackup info about systemdns backup


This comment doesn't seem to be necessary.

Suggested change

// systemDNSBackup info about systemdns backup

jyyi1 · 2024-05-01T20:22:54Z

x/examples/outline-cli/dns_linux.go

-		return err
+	err := backupAndWriteFile(resolvConfFile, resolvConfBackupFile, setting)
+	if err != nil {
+		return fmt.Errorf("dns backup: %w", err)


I'd rather simply return the error, because we have provided enough error details in backupAndWriteFile function.

Suggested change

return fmt.Errorf("dns backup: %w", err)

return err

jyyi1 · 2024-05-01T20:23:13Z

x/examples/outline-cli/dns_linux.go

-	restoreFileIfExists(resolvConfHeadBackupFile, resolvConfHeadFile)
+	err = backupAndWriteFile(resolvConfHeadFile, resolvConfHeadBackupFile, setting)
+	if err != nil {
+		return fmt.Errorf("dns head backup: %w", err)


Similar to above.

Suggested change

return fmt.Errorf("dns head backup: %w", err)

return err

Also if we are simply returning the err here, we can just do return backupAndWriteFile(...) directly.

jyyi1 · 2024-05-01T20:27:00Z

x/examples/outline-cli/dns_linux.go

+			return fmt.Errorf("failed to backup DNS config file '%s' to '%s': %w", original, backup, err)
+		}
+	} else if !errors.Is(err, os.ErrNotExist) { // if not exist - it's ok, just write to it
+		return errors.New("check original file exists")


Suggested change

return errors.New("check original file exists")

return fmt.Errorf("failed to check the existence of DNS config file '%s': %w", original, err)

jyyi1 · 2024-05-01T20:28:23Z

x/examples/outline-cli/dns_linux.go

+
+	systemDNSBackups = append(systemDNSBackups, systemDNSBackup{
+		original: original,
+		backup:   backup,
+	})
+


I'd suggest to put these lines before os.WriteFile. Because we have already backed up the original DNS config, even though we failed to write to the new file, we still need to copy the backup file back to original when we try to reset the DNS config.

jyyi1 · 2024-05-01T20:29:57Z

x/examples/outline-cli/dns_linux.go

+			}
+			logging.Info.Printf("DNS config removed '%s'\n", backup.original)
+		} else {
+			logging.Err.Printf("checking backup '%s': %v", backup.backup, err)


Suggested change

logging.Err.Printf("checking backup '%s': %v", backup.backup, err)

logging.Err.Printf("failed to check the existence of DNS config backup file '%s': %v\n", backup.backup, err)

jyyi1 · 2024-05-01T20:31:25Z

x/examples/outline-cli/dns_linux.go

+		} else if errors.Is(err, os.ErrNotExist) {
+			// backup not exist - just remove original, because it's created by ourselves
+			if err := os.Remove(backup.original); err != nil {
+				logging.Err.Printf("removing original DNS config file '%s': %v", backup.original, err)


Suggested change

logging.Err.Printf("removing original DNS config file '%s': %v", backup.original, err)

logging.Err.Printf("failed to remove Outline DNS config file '%s': %v\n", backup.original, err)

jyyi1 · 2024-05-01T20:32:19Z

x/examples/outline-cli/dns_linux.go

+				logging.Err.Printf("removing original DNS config file '%s': %v", backup.original, err)
+				continue
+			}
+			logging.Info.Printf("DNS config removed '%s'\n", backup.original)


Suggested change

logging.Info.Printf("DNS config removed '%s'\n", backup.original)

logging.Info.Printf("Outline DNS config '%s' has been removed\n", backup.original)

jyyi1 · 2024-05-01T20:33:21Z

x/examples/outline-cli/ipv6_linux.go

@@ -40,7 +40,7 @@ func enableIPv6(enabled bool) (bool, error) {
 	} else {
 		disabledStr[0] = '1'
 	}
-	if err := os.WriteFile(disableIPv6ProcFile, disabledStr, 0644); err != nil {
+	if err := os.WriteFile(disableIPv6ProcFile, disabledStr, 0o644); err != nil {


👍! Thanks.

We can keep this change. Though it's not related to DNS, but it's CLI.

jyyi1

Thanks @zeslava , the code looks good to me!

jyyi1 · 2024-05-30T02:24:55Z

Hi @zeslava , on last thing, could you fetch the latest main and merge it to this PR? It will unblock the failing CI so I can merge this PR. Thanks.

Update dns_linux.go

7b7f769

It's ok if no original resolv.conf file found

fortuna requested a review from jyyi1 March 25, 2024 18:01

jyyi1 approved these changes Apr 1, 2024

View reviewed changes

jyyi1 requested changes Apr 1, 2024

View reviewed changes

jyyi1 changed the title ~~Update dns_linux.go~~ example(outline-cli): configure system DNS when resolv.conf does not exist Apr 1, 2024

fix: example/outline-cli backup/restore dns files

e4c931c

jyyi1 requested changes Apr 29, 2024

View reviewed changes

fix: simplify examples/outline-cli/systemDNSBackup logic + gofumpt

fb2aa24

zeslava added 5 commits April 30, 2024 23:18

fix: examples/outline-cli/restoreSystemDNSServer loop

1601dfb

Merge branch 'main' into patch-1

a52712a

Merge branch 'main' into patch-1

8178ee5

Merge remote-tracking branch 'origin/patch-1' into patch-1

409d92f

fix: transport/tlsfrag/tls.go fmt

3ba1f81

jyyi1 requested changes May 1, 2024

View reviewed changes

jyyi1 mentioned this pull request May 28, 2024

Any way to run Outline client on linux server terminal by command line? #194

Closed

zeslava added 3 commits May 29, 2024 00:04

fix: revert style changes

5b1fa30

Merge branch 'Jigsaw-Code:main' into patch-1

6dc82c9

fix: revert

72f5d05

jyyi1 approved these changes May 28, 2024

View reviewed changes

jyyi1 mentioned this pull request May 28, 2024

fix(test): wrong url used in expired certificate test #242

Merged

Merge branch 'Jigsaw-Code:main' into patch-1

b17a719

jyyi1 merged commit c99a7ce into Jigsaw-Code:main May 30, 2024
4 checks passed

Kir-Antipov mentioned this pull request Jul 4, 2024

Failed to configure system DNS Kir-Antipov/outline-cli#1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

example(outline-cli): configure system DNS when `resolv.conf` does not exist #203

example(outline-cli): configure system DNS when `resolv.conf` does not exist #203

zeslava commented Mar 23, 2024

jyyi1 left a comment

jyyi1 left a comment •

edited

Loading

jyyi1 left a comment

jyyi1 Apr 29, 2024

jyyi1 Apr 29, 2024 •

edited

Loading

jyyi1 Apr 29, 2024

jyyi1 Apr 29, 2024

jyyi1 Apr 29, 2024

jyyi1 Apr 29, 2024 •

edited

Loading

zeslava commented Apr 30, 2024

jyyi1 left a comment

jyyi1 May 1, 2024

zeslava May 28, 2024

jyyi1 May 1, 2024

jyyi1 May 1, 2024

jyyi1 May 1, 2024

jyyi1 May 1, 2024

jyyi1 May 1, 2024

jyyi1 May 1, 2024

zeslava May 28, 2024

jyyi1 May 1, 2024 •

edited

Loading

jyyi1 May 1, 2024

jyyi1 May 1, 2024

jyyi1 May 1, 2024

jyyi1 May 1, 2024

jyyi1 left a comment

jyyi1 commented May 30, 2024

	func backupAndWriteFile(original, backup string, data []byte) (restoreBackup, error) {
	func backupAndWriteFile(original, backup string, data []byte) error {

	return errors.New("check original file exists")
	return fmt.Errorf("failed to check the existence of DNS config file '%s': %w", original, err)

	logging.Err.Printf("checking backup '%s': %v", backup.backup, err)
	logging.Err.Printf("failed to check the existence of DNS config backup file '%s': %v\n", backup.backup, err)

	logging.Err.Printf("removing original DNS config file '%s': %v", backup.original, err)
	logging.Err.Printf("failed to remove Outline DNS config file '%s': %v\n", backup.original, err)

	logging.Info.Printf("DNS config removed '%s'\n", backup.original)
	logging.Info.Printf("Outline DNS config '%s' has been removed\n", backup.original)

example(outline-cli): configure system DNS when resolv.conf does not exist #203

example(outline-cli): configure system DNS when resolv.conf does not exist #203

Conversation

zeslava commented Mar 23, 2024

jyyi1 left a comment

Choose a reason for hiding this comment

jyyi1 left a comment • edited Loading

Choose a reason for hiding this comment

jyyi1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyyi1 Apr 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyyi1 Apr 29, 2024 • edited Loading

Choose a reason for hiding this comment

zeslava commented Apr 30, 2024

jyyi1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyyi1 May 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyyi1 left a comment

Choose a reason for hiding this comment

jyyi1 commented May 30, 2024

example(outline-cli): configure system DNS when `resolv.conf` does not exist #203

example(outline-cli): configure system DNS when `resolv.conf` does not exist #203

jyyi1 left a comment •

edited

Loading

jyyi1 Apr 29, 2024 •

edited

Loading

jyyi1 Apr 29, 2024 •

edited

Loading

jyyi1 May 1, 2024 •

edited

Loading