Skip to content

Conversation

@mrnugget
Copy link
Contributor

This fixes #429 by switching src validate to use yaml.v3.

When trying to confirm that it still works, I found out that parsing the validate input as YAML didn't work for me, even without the change.

Both yaml.v3 and yaml.v2 assume that the fields in the YAML are in snake_case and need tags added to the struct fields to make it work with camelCase.

The github.com/ghodss/yaml works with snake_case and also supports the unmarshalling into a *json.RawMessage, which yaml.v2 and yaml.v3 don't support.

But since the goal was to only use a single YAML library I switched to yaml.v3 and changed the code to work with it. I tested this locally and checked that everything's unmarshalled correctly.'

See the script here for how I explored this (and learned to curse YAML yet again).

package main

import (
	"encoding/json"
	"fmt"

	ghodssyaml "github.com/ghodss/yaml"
	yamlv2 "gopkg.in/yaml.v2"
	yamlv3 "gopkg.in/yaml.v3"
)

type unmarshalFunc func([]byte, interface{}) error

func main() {
	funcs := []struct {
		name string
		fn   unmarshalFunc
	}{
		{name: "yaml.v2", fn: yamlv2.Unmarshal},
		{name: "yaml.v3", fn: yamlv3.Unmarshal},
		{name: "ghodss/yaml", fn: ghodssyaml.Unmarshal},
	}

	fmt.Printf("validationSpec:\n")
	for _, tt := range funcs {
		fmt.Printf("--- %s ---\n", tt.name)
		var vspec validationSpec
		if err := tt.fn([]byte(input), &vspec); err != nil {
			fmt.Printf("%s error: %s\n", tt.name, err)
		}
		if have, want := vspec.FirstAdmin.Email, "foo@example.com"; have != want {
			fmt.Printf("%s vspec.FirstAdmin.Email wrong. want=%q, have=%q\n", tt.name, want, have)
		}
		fmt.Printf("done.\n")
	}

	fmt.Printf("\n\nvalidationSpecWithCamelCaseTags:\n")
	for _, tt := range funcs {
		fmt.Printf("--- %s ---\n", tt.name)
		var vspec validationSpecWithCamelCaseTags
		if err := tt.fn([]byte(input), &vspec); err != nil {
			fmt.Printf("%s error: %s\n", tt.name, err)
		}
		if have, want := vspec.FirstAdmin.Email, "foo@example.com"; have != want {
			fmt.Printf("%s vspec.FirstAdmin.Email wrong. want=%q, have=%q\n", tt.name, want, have)
		}
		fmt.Printf("done.\n")
	}
}

type validationSpec struct {
	FirstAdmin struct {
		Email             string
		Username          string
		Password          string
		CreateAccessToken bool
	}
	WaitRepoCloned struct {
		Repo                     string
		MaxTries                 int
		SleepBetweenTriesSeconds int
	}
	SearchQuery     string
	ExternalService struct {
		Kind           string
		DisplayName    string
		Config         *json.RawMessage
		DeleteWhenDone bool
	}
}

type validationSpecWithCamelCaseTags struct {
	FirstAdmin struct {
		Email             string `yaml:"email"`
		Username          string `yaml:"username"`
		Password          string `yaml:"password"`
		CreateAccessToken bool   `yaml:"createAccessToken"`
	} `yaml:"firstAdmin"`
	WaitRepoCloned struct {
		Repo                     string `yaml:"repo"`
		MaxTries                 int    `yaml:"maxTries"`
		SleepBetweenTriesSeconds int    `yaml:"sleepBetweenTriesSecond"`
	} `yaml:"waitRepoCloned"`
	SearchQuery     string `yaml:"searchQuery"`
	ExternalService struct {
		Kind           string                 `yaml:"kind"`
		DisplayName    string                 `yaml:"displayName"`
		Config         map[string]interface{} `yaml:"config"`
		DeleteWhenDone bool                   `yaml:"deleteWhenDone"`
	} `yaml:"externalService"`
}

const input = `# creates the first admin user on a fresh install (skips creation if user exists)
firstAdmin:
    email: foo@example.com
    username: foo
    password: "{{ .admin_password }}"

# adds the specified code host
externalService:
  config:
    url: https://github.com
    token: "{{ .github_token }}"
    orgs: []
    repos:
      - sourcegraph-testing/zap
  kind: GITHUB
  displayName: footest
  # set to true if this code host config should be deleted at the end of validation
  deleteWhenDone: true

# checks maxTries if specified repo is cloned and waits sleepBetweenTriesSeconds between checks 
waitRepoCloned:
  repo: github.com/footest/foo
  maxTries: 5
  sleepBetweenTriesSeconds: 2

# performs the specified search and checks that at least one result is returned
searchQuery: repo:^github.com/footest/foo$ uniquelyFoo`

That prints:

validationSpec:
--- yaml.v2 ---
yaml.v2 vspec.FirstAdmin.Email wrong. want="foo@example.com", have=""
done.
--- yaml.v3 ---
yaml.v3 vspec.FirstAdmin.Email wrong. want="foo@example.com", have=""
done.
--- ghodss/yaml ---
done.


validationSpecWithCamelCaseTags:
--- yaml.v2 ---
done.
--- yaml.v3 ---
done.
--- ghodss/yaml ---
done.

@mrnugget mrnugget requested review from a team and uwedeportivo January 19, 2021 10:41
This fixes #429 by switching `src validate` to use `yaml.v3`.

When trying to confirm that it still works, I found out that parsing the
validate input as YAML didn't work for me, even without the change.

Both `yaml.v3` and `yaml.v2` assume that the fields in the YAML are in
`snake_case` and need tags added to the struct fields to make it work
with `camelCase`.

The `github.com/ghodss/yaml` works with `snake_case` and also supports
the unmarshalling into a `*json.RawMessage`, which `yaml.v2` and
`yaml.v3` don't support.

But since the goal was to only use a single YAML library I switched to
`yaml.v3` and changed the code to work with it. I tested this locally
and checked that everything's unmarshalled correctly.
Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

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

Oh 👀 A linter rule for checking that yaml fields have tags would be nice 😬

@mrnugget mrnugget merged commit 5fe51e6 into main Jan 19, 2021
@mrnugget mrnugget deleted the mrn/only-yaml-v3 branch January 19, 2021 12:35
scjohns pushed a commit that referenced this pull request Apr 24, 2023
This fixes #429 by switching `src validate` to use `yaml.v3`.

When trying to confirm that it still works, I found out that parsing the
validate input as YAML didn't work for me, even without the change.

Both `yaml.v3` and `yaml.v2` assume that the fields in the YAML are in
`snake_case` and need tags added to the struct fields to make it work
with `camelCase`.

The `github.com/ghodss/yaml` works with `snake_case` and also supports
the unmarshalling into a `*json.RawMessage`, which `yaml.v2` and
`yaml.v3` don't support.

But since the goal was to only use a single YAML library I switched to
`yaml.v3` and changed the code to work with it. I tested this locally
and checked that everything's unmarshalled correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Only use a single yaml library.

3 participants