Skip to content

Commit

Permalink
Clean up root directory to conform to standard go spec. (#35)
Browse files Browse the repository at this point in the history
- There's no need to make separate packages for the zap internals, as it
  would be just a single file + test per package.
- We have to update the build commands to specify the cmd dir.
  • Loading branch information
issmirnov authored Dec 7, 2020
1 parent bf4e76f commit 0ce9172
Show file tree
Hide file tree
Showing 11 changed files with 132 additions and 113 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ dist/
# compiled files
zap

# Include our actual code, which is excluded by the "zap" directive above.
!/cmd/zap

# Goland
.idea/*

Expand Down
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ os:
script:
- diff -u <(echo -n) <(go fmt $(go list ./...))
- go vet $(go list ./...)
- go test -short -v ./... -race -coverprofile=coverage.txt -covermode=atomic
- go build -v ./...
- go test -short -v ./... -race -coverprofile=coverage.txt -covermode=atomic ./cmd
- go build -o zap -v ./cmd/
- ./e2e.sh

# calls goreleaser
Expand Down
8 changes: 4 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
## Contributing to Zap

Patches are welcome! Please use the standard GitHub workflow - fork this
repo and submit a PR. I'll usually get to it within a few days.
repo and submit a PR. I'll usually get to it within a few days. If I miss your PR email, feel free to ping me directly at isgsmirnov@gmail.com after about a week.

## Setting up dev environment

Expand All @@ -13,16 +13,16 @@ or your favorite web editor with Golang support.
git clone $your_fork zap
cd zap
go get
go build . # sanity check
go build -o zap -v ./cmd/ # sanity check
# install test deps and run all tests
go test ./... -v
go test -short -v ./... -race -coverprofile=coverage.txt -covermode=atomic ./cmd
./e2e.sh
```

## Handy commands for local development:

- `go build && ./zap` to run locally
- `go build -o zap -v ./cmd/ && ./zap` to run locally
- `curl -I -L -H 'Host: g' localhost:8927/z` - to test locally e2e
- `goconvey -excludedDirs dist` - launches web UI for go tests.
- `./e2e.sh` runs CLI tests.
Expand Down
36 changes: 21 additions & 15 deletions main.go → cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"os"
"path"

"github.com/issmirnov/zap/cmd/zap"

"github.com/fsnotify/fsnotify"

"github.com/julienschmidt/httprouter"
Expand All @@ -34,24 +36,24 @@ func main() {
}

// load config for first time.
c, err := parseYaml(*configName)
c, err := zap.ParseYaml(*configName)
if err != nil {
log.Printf("Error parsing config file. Please fix syntax: %s\n", err)
return
}

// Perform extended validation of config.
if *validate {
if err := validateConfig(c); err != nil {
if err := zap.ValidateConfig(c); err != nil {
fmt.Println(err.Error())
os.Exit(1)
}
fmt.Println("No errors detected.")
os.Exit(0)
}

context := &context{config: c}
updateHosts(context) // sync changes since last run.
context := &zap.Context{Config: c}
zap.UpdateHosts(context) // sync changes since last run.

// Enable hot reload.
watcher, err := fsnotify.NewWatcher()
Expand All @@ -60,26 +62,30 @@ func main() {
}
defer watcher.Close()

// Enable hot reload.
cb := makeCallback(context, *configName)
go watchChanges(watcher, *configName, cb)
cb := zap.MakeReloadCallback(context, *configName)
go zap.WatchConfigFileChanges(watcher, *configName, cb)
err = watcher.Add(path.Dir(*configName))
if err != nil {
log.Fatal(err)
}

// Set up routes.
router := SetupRouter(context)

// TODO check for errors - addr in use, sudo issues, etc.
fmt.Printf("Launching %s on %s:%d\n", appName, *host, *port)
log.Fatal(http.ListenAndServe(fmt.Sprintf("%s:%d", *host, *port), router))
}

func SetupRouter(context *zap.Context) *httprouter.Router {
router := httprouter.New()
router.Handler("GET", "/", ctxWrapper{context, IndexHandler})
router.Handler("GET", "/varz", ctxWrapper{context, VarsHandler})
router.HandlerFunc("GET", "/healthz", HealthHandler)
router.Handler("GET", "/", zap.CtxWrapper{Context: context, H: zap.IndexHandler})
router.Handler("GET", "/varz", zap.CtxWrapper{Context: context, H: zap.VarsHandler})
router.HandlerFunc("GET", "/healthz", zap.HealthHandler)

// https://github.com/julienschmidt/httprouter is having issues with
// wildcard handling. As a result, we have to register index handler
// as the fallback. Fix incoming.
router.NotFound = ctxWrapper{context, IndexHandler}

// TODO check for errors - addr in use, sudo issues, etc.
fmt.Printf("Launching %s on %s:%d\n", appName, *host, *port)
log.Fatal(http.ListenAndServe(fmt.Sprintf("%s:%d", *host, *port), router))
router.NotFound = zap.CtxWrapper{Context: context, H: zap.IndexHandler}
return router
}
48 changes: 25 additions & 23 deletions config.go → cmd/zap/config.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package main
package zap

import (
"bytes"
Expand Down Expand Up @@ -47,8 +47,8 @@ func parseYamlString(config string) (*gabs.Container, error) {
return j, nil
}

// parseYaml takes a file name and returns a gabs config object.
func parseYaml(fname string) (*gabs.Container, error) {
// ParseYaml takes a file name and returns a gabs Config object.
func ParseYaml(fname string) (*gabs.Container, error) {
data, err := Afero.ReadFile(fname)
if err != nil {
fmt.Printf("Unable to read file: %s\n", err.Error())
Expand All @@ -63,10 +63,10 @@ func parseYaml(fname string) (*gabs.Container, error) {
return j, nil
}

// validateConfig verifies that there are no unexpected values in the config file.
// at each level of the config, we should either have a KV for expansions, or a leaf node
// ValidateConfig verifies that there are no unexpected values in the Config file.
// at each level of the Config, we should either have a KV for expansions, or a leaf node
// with the values oneof "expand", "query", "ssl_off" that map to a string.
func validateConfig(c *gabs.Container) error {
func ValidateConfig(c *gabs.Container) error {
var errors *multierror.Error
children := c.ChildrenMap()
seenKeys := make(map[string]struct{})
Expand Down Expand Up @@ -105,15 +105,17 @@ func validateConfig(c *gabs.Container) error {
errors = multierror.Append(errors, fmt.Errorf("unexpected string value under key %s, got: %v", k, v.Data()))
}
// recurse, collect any errors.
if err := validateConfig(v); err != nil {
if err := ValidateConfig(v); err != nil {
errors = multierror.Append(errors, err)
}
}
}
return errors.ErrorOrNil()
}

func watchChanges(watcher *fsnotify.Watcher, fname string, cb func()) {
// WatchConfigFileChanges will attach an fsnotify watcher to the config file, and trigger
// the cb function when the file is updated.
func WatchConfigFileChanges(watcher *fsnotify.Watcher, fname string, cb func()) {
for {
select {
case event := <-watcher.Events:
Expand All @@ -133,22 +135,22 @@ func watchChanges(watcher *fsnotify.Watcher, fname string, cb func()) {
}

// TODO: add tests. simulate touching a file.
// updateHosts will attempt to write the zap list of shortcuts
// UpdateHosts will attempt to write the zap list of shortcuts
// to /etc/hosts. It will gracefully fail if there are not enough
// permissions to do so.
func updateHosts(c *context) {
func UpdateHosts(c *Context) {
hostPath := "/etc/hosts"

// 1. read file, prep buffer.
data, err := ioutil.ReadFile(hostPath)
if err != nil {
log.Println("open config: ", err)
log.Println("open Config: ", err)
}
var replacement bytes.Buffer

// 2. generate payload.
replacement.WriteString(delimStart)
children := c.config.ChildrenMap()
children := c.Config.ChildrenMap()
for k := range children {
replacement.WriteString(fmt.Sprintf("127.0.0.1 %s\n", k))
}
Expand All @@ -170,27 +172,27 @@ func updateHosts(c *context) {
}
}

// makeCallback returns a func that that updates global state.
func makeCallback(c *context, configName string) func() {
// MakeReloadCallback returns a func that that reads the config file and updates global state.
func MakeReloadCallback(c *Context, configName string) func() {
return func() {
data, err := parseYaml(configName)
data, err := ParseYaml(configName)
if err != nil {
log.Printf("Error loading new config: %s. Fallback to old config.", err)
log.Printf("Error loading new Config: %s. Fallback to old Config.", err)
return
}
err = validateConfig(data)
err = ValidateConfig(data)
if err != nil {
log.Printf("Error validating new config: %s. Fallback to old config.", err)
log.Printf("Error validating new Config: %s. Fallback to old Config.", err)
return
}

// Update config atomically
c.configMtx.Lock()
c.config = data
c.configMtx.Unlock()
// Update Config atomically
c.ConfigMtx.Lock()
c.Config = data
c.ConfigMtx.Unlock()

// Sync DNS entries.
updateHosts(c)
UpdateHosts(c)
return
}
}
22 changes: 11 additions & 11 deletions config_test.go → cmd/zap/config_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package main
package zap

import (
"testing"
Expand Down Expand Up @@ -120,8 +120,8 @@ func TestParseYaml(t *testing.T) {
Convey("Given a valid 'c.yml' file", t, func() {
Afero = &afero.Afero{Fs: afero.NewMemMapFs()}
Afero.WriteFile("c.yml", []byte(cYaml), 0644)
c, err := parseYaml("c.yml")
Convey("parseYaml should throw no error", func() {
c, err := ParseYaml("c.yml")
Convey("ParseYaml should throw no error", func() {
So(err, ShouldBeNil)
})
Convey("the gabs object should have path 'zz' present", func() {
Expand All @@ -131,33 +131,33 @@ func TestParseYaml(t *testing.T) {
}

func TestValidateConfig(t *testing.T) {
Convey("Given a correctly formatted yaml config", t, func() {
Convey("Given a correctly formatted yaml Config", t, func() {
conf, _ := parseYamlString(cYaml)
//fmt.Printf(err.Error())
Convey("The validator should pass", func() {
So(validateConfig(conf), ShouldBeNil)
So(ValidateConfig(conf), ShouldBeNil)
})
})

// The YAML libraries don't have support for detecting duplicate keys
// at parse time. Users will have to figure this out themselves.
//Convey("Given a yaml config with duplicated keys", t, func() {
//Convey("Given a yaml Config with duplicated keys", t, func() {
// conf, _ := parseYamlString(duplicatedYAML)
// Convey("The validator should complain", func() {
// So(validateConfig(conf), ShouldNotBeNil)
// So(ValidateConfig(conf), ShouldNotBeNil)
// })
//})

Convey("Given a YAML config with unknown keys", t, func() {
Convey("Given a YAML Config with unknown keys", t, func() {
conf, _ := parseYamlString(badkeysYAML)
Convey("The validator should raise an error", func() {
So(validateConfig(conf), ShouldNotBeNil)
So(ValidateConfig(conf), ShouldNotBeNil)
})
})

Convey("Given a YAML config with malformed values", t, func() {
Convey("Given a YAML Config with malformed values", t, func() {
conf, _ := parseYamlString(badValuesYAML)
err := validateConfig(conf)
err := ValidateConfig(conf)
Convey("The validator should raise a ton of errors", func() {
So(err, ShouldNotBeNil)
So(err.Error(), ShouldContainSubstring, "expected float64 value for string, got: not_int")
Expand Down
35 changes: 35 additions & 0 deletions cmd/zap/structs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package zap

import (
"fmt"
"net/http"
"sync"

"github.com/Jeffail/gabs/v2"
)

type Context struct {
// Json container with path configs
Config *gabs.Container

// Enables safe hot reloading of Config.
ConfigMtx sync.Mutex
}

type CtxWrapper struct {
*Context
H func(*Context, http.ResponseWriter, *http.Request) (int, error)
}

func (cw CtxWrapper) ServeHTTP(w http.ResponseWriter, r *http.Request) {
status, err := cw.H(cw.Context, w, r) // this runs the actual handler, defined in struct.
if err != nil {
switch status {
case http.StatusInternalServerError:
http.Error(w, fmt.Sprintf("HTTP %d: %q", status, err), status)
// TODO - add bad request?
default:
http.Error(w, err.Error(), status)
}
}
}
8 changes: 4 additions & 4 deletions text.go → cmd/zap/text.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package main
package zap

import (
"bytes"
Expand Down Expand Up @@ -59,11 +59,11 @@ func getPrefix(c *gabs.Container) (string, int, error) {
return "", 0, fmt.Errorf("casting port key to float64 failed for %T:%v", p, p)
}

return "", 0, fmt.Errorf("error in config, no key matching 'expand', 'query', 'port' or 'schema' in %s", c.String())
return "", 0, fmt.Errorf("error in Config, no key matching 'expand', 'query', 'port' or 'schema' in %s", c.String())
}

// expandPath takes a config, list of tokens (parsed from request) and the results buffer
// At each level of recursion, it matches the token to the action described in the config, and writes it
// expandPath takes a Config, list of tokens (parsed from request) and the results buffer
// At each level of recursion, it matches the token to the action described in the Config, and writes it
// to the result buffer. There is special care needed to handle slashes correctly, which makes this function
// quite nontrivial. Tests are crucial to ensure correctness.
func expandPath(c *gabs.Container, token *list.Element, res *bytes.Buffer) {
Expand Down
2 changes: 1 addition & 1 deletion text_test.go → cmd/zap/text_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package main
package zap

import (
"bytes"
Expand Down
Loading

0 comments on commit 0ce9172

Please sign in to comment.