redesign or refactor pkg/definition

[kevindrosendahl]

pkg/definition currently has a number of design shortcomings, among them:

• coupling of v1.Reference to resolver.ComponentResolver
  • currently the component resolver is capable of resolving v1.References
  • in the future will we need to be able to switch on different versions of references?
  • are references part of a version of the language, or something more inherent? currently it's a little bit of both
• coupling of v1.SecretRef to $secret
  • currently on template evaluation if an lval is $secret, we transform it into a *pkg/definition/v1.SecretRef:

    lattice/pkg/definition/template/template.go

    Lines 113 to 141 in 662e7d9

    	if k == SecretParameterLVal {
    	// Ensure the $secret key is a string
    	// TODO(kevindrosendahl): validate character set here?
    	name, ok := v.(string)
    	if !ok {
    	return nil, &ParameterTypeError{
    	Expected: string(ParameterTypeSecret),
    	Actual: reflect.TypeOf(v).String(),
    	}
    	}
    	secretRefPath, err := tree.NewPathSubcomponentFromParts(path, name)
    	if err != nil {
    	return nil, err
    	}
    	secretRef := &definitionv1.SecretRef{
    	Value: secretRefPath,
    	}
    	return secretRef, nil
    	}
    	result, err := evaluateValue(path, v, bindings)
    	if err != nil {
    	return nil, err
    	}
    	val[k] = result
    	}

  • this will couple future versions of the definition language to v1.SecretRefs
• no clear structural differentiation between the user-accessible definition language and the resolved definition structs
  • pkg/definition/v1 contains structs describing the definitions possible end states, as well as some of the initial (pre-resolution) states, but not all of the initial states, as well as no way to determine what is initial state vs end state
    • pkg/definition/v1 contains the Reference type, but a Reference can't exist after resolution
    • pkg/definition/v1 only contains a SecretRef type, but a user cannot create a SecretRef, only a $secret
  • in addition to it just being confusing, it also eliminates some of the utility in e.g. automatically generating docs
• should pkg/definition/resolver and pkg/definition/component be merged?
• resolution info was hastily put together
  • I do think the general idea of ResolveReference returning a *ResolutionResult is a good idea, however it should probably be re-examined and probably expanded
  • currently ResolutionNodeInfo has json annotations:

    lattice/pkg/definition/resolver/component_resolver.go

    Lines 41 to 44 in 662e7d9

    	type ResolutionNodeInfo struct {
    	Commit git.CommitReference `json:"commit"`
    	SSHKeySecret *tree.PathSubcomponent `json:"sshKeySecret,omitempty"`
    	}

    solely because we know that we serialize it later in a k8s custom resource. this is pretty leaky
  • *ResolutionResult could probably be expanded to contain more information about how exactly different values were derived (e.g. the value of components[0].exec.env['foo'] came from parameter bar).
  • similarly, a lot more information could be returned about parsing errors, etc