Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

combine xpkg auth ext anno during build #309

Merged
merged 5 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions cmd/up/xpkg/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package xpkg

import (
"context"
"io"
"path/filepath"

"github.com/crossplane/crossplane-runtime/pkg/errors"
Expand All @@ -36,8 +37,6 @@ const (
errBuildPackage = "failed to build package"
errImageDigest = "failed to get package digest"
errCreatePackage = "failed to create package file"

examplesDir = "examples/"
)

// AfterApply constructs and binds Upbound-specific context to any subcommands
Expand All @@ -56,6 +55,18 @@ func (c *buildCmd) AfterApply() error {
return err
}

var authBE parser.Backend
if ax, err := filepath.Abs(c.AuthExt); err == nil {
if axf, err := c.fs.Open(ax); err == nil {
defer func() { _ = axf.Close() }()
b, err := io.ReadAll(axf)
if err != nil {
return err
}
authBE = parser.NewEchoBackend(string(b))
}
}

pp, err := yaml.New()
if err != nil {
return err
Expand All @@ -68,8 +79,9 @@ func (c *buildCmd) AfterApply() error {
parser.FsFilters(
append(
buildFilters(root, c.Ignore),
xpkg.SkipContains(examplesDir))...),
xpkg.SkipContains(c.ExamplesRoot), xpkg.SkipContains(c.AuthExt))...),
),
authBE,
parser.NewFsBackend(
c.fs,
parser.FsDir(ex),
Expand Down Expand Up @@ -99,6 +111,7 @@ type buildCmd struct {
Controller string `help:"Controller image used as base for package."`
PackageRoot string `short:"f" help:"Path to package directory." default:"."`
ExamplesRoot string `short:"e" help:"Path to package examples directory." default:"./examples"`
AuthExt string `short:"a" help:"Path to an authentication extension file." default:"auth.yaml"`
Ignore []string `help:"Paths, specified relative to --package-root, to exclude from the package."`
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ require (
github.com/willabides/kongplete v0.3.0
golang.org/x/sync v0.0.0-20220601150217-0de741cfad7f
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211
gopkg.in/yaml.v2 v2.4.0
gopkg.in/yaml.v3 v3.0.1
helm.sh/helm/v3 v3.9.0
k8s.io/api v0.24.3
Expand Down Expand Up @@ -217,7 +218,6 @@ require (
google.golang.org/protobuf v1.28.0 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
k8s.io/apiserver v0.24.3 // indirect
k8s.io/cli-runtime v0.24.3 // indirect
k8s.io/component-base v0.24.3 // indirect
Expand Down
121 changes: 106 additions & 15 deletions internal/xpkg/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,28 +23,39 @@ import (

"github.com/crossplane/crossplane-runtime/pkg/errors"
pkgmetav1 "github.com/crossplane/crossplane/apis/pkg/meta/v1"
v1alpha1 "github.com/crossplane/crossplane/apis/pkg/meta/v1alpha1"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/empty"
"github.com/google/go-containerregistry/pkg/v1/mutate"
"gopkg.in/yaml.v2"
crd "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer/json"

"github.com/crossplane/crossplane-runtime/pkg/parser"

"github.com/upbound/up/internal/xpkg/parser/examples"
"github.com/upbound/up/internal/xpkg/parser/linter"
"github.com/upbound/up/internal/xpkg/scheme"
)

const (
errParserPackage = "failed to parse package"
errParserExample = "failed to parse examples"
errLintPackage = "failed to lint package"
errInitBackend = "failed to initialize package parsing backend"
errTarFromStream = "failed to build tarball from stream"
errLayerFromTar = "failed to convert tarball to image layer"
errDigestInvalid = "failed to get digest from image layer"
errBuildImage = "failed to build image from layers"
errConfigFile = "failed to get config file from image"
errMutateConfig = "failed to mutate config for image"
errParserPackage = "failed to parse package"
errParserExample = "failed to parse examples"
errLintPackage = "failed to lint package"
errInitBackend = "failed to initialize package parsing backend"
errTarFromStream = "failed to build tarball from stream"
errLayerFromTar = "failed to convert tarball to image layer"
errDigestInvalid = "failed to get digest from image layer"
errBuildImage = "failed to build image from layers"
errConfigFile = "failed to get config file from image"
errMutateConfig = "failed to mutate config for image"
errBuildObjectScheme = "failed to build scheme for package encoder"
errParseAuth = "an auth extension was supplied but could not be parsed"
errAuthNotAnnotated = "an auth extension was supplied but but the " + ProviderConfigKind + " object could not be found"
authMetaAnno = "auth.upbound.io/group"
authObjectAnno = "auth.upbound.io/config"
ProviderConfigKind = "ProviderConfig"
)

// annotatedTeeReadCloser is a copy of io.TeeReader that implements
Expand Down Expand Up @@ -90,15 +101,17 @@ func (t *teeReader) Annotate() any {
type Builder struct {
pb parser.Backend
eb parser.Backend
ab parser.Backend

pp parser.Parser
ep *examples.Parser
}

// New returns a new Builder.
func New(pkg, ex parser.Backend, pp parser.Parser, ep *examples.Parser) *Builder {
func New(pkg, ab, ex parser.Backend, pp parser.Parser, ep *examples.Parser) *Builder {
return &Builder{
pb: pkg,
ab: ab,
eb: ex,
pp: pp,
ep: ep,
Expand All @@ -120,6 +133,20 @@ func WithController(img v1.Image) BuildOpt {
}
}

type AuthExtension struct {
Version string `yaml:"version"`
Discriminant string `yaml:"discriminant"`
Sources []struct {
Name string `yaml:"name"`
Docs string `yaml:"docs"`
AdditionalResources []struct {
Type string `yaml:"type"`
Ref string `yaml:"ref"`
} `yaml:"additionalResources"`
ShowFields []string `yaml:"showFields"`
} `yaml:"sources"`
}

// Build compiles a Crossplane package from an on-disk package.
func (b *Builder) Build(ctx context.Context, opts ...BuildOpt) (v1.Image, runtime.Object, error) { // nolint:gocyclo
bOpts := &buildOpts{
Expand Down Expand Up @@ -149,9 +176,7 @@ func (b *Builder) Build(ctx context.Context, opts ...BuildOpt) (v1.Image, runtim
examplesExist = false
}

// Copy stream once to parse and once write to tarball.
pkgBuf := new(bytes.Buffer)
pkg, err := b.pp.Parse(ctx, annotatedTeeReadCloser(pkgReader, pkgBuf))
pkg, err := b.pp.Parse(ctx, pkgReader)
Comment on lines -154 to +179
Copy link
Contributor

Choose a reason for hiding this comment

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

One option to avoid having to re-serialize all objects in the package would be to augment the annotatedTeeReadCloser to skip writing the ProviderConfig to the pkgBuf but store the contents so that they can be augmented and re-written to the end of the buffer prior to linting. That would also avoid any modification of content that was not going through the "pre-processor".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could work but would require us to search a byte array rather than an already parsed object to check their Kind and then find the beginning and end of that object to extract and place at the end which seems like a much more problematic way to do it. Yes, the current method will cause a checksum change between identical packages post up release but only once going forward. The re-serialization of objects here gives us much greater flexibility in mutating objects here and in the future as needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the current method will cause a checksum change between identical packages post up release but only once going forward. The re-serialization of objects here gives us much greater flexibility in mutating objects here and in the future as needed.

This seems like a very reasonable outlook to me and I am comfortable with different versions of up producing varying package content. It is similar to any compiler -- the produced artifact from one version to the next should be valid in respect to how it "runs" and any contract established (such as FFI), but it doesn't need to be identical. We are producing valid xpkgs here so I think your point is a good one 👍🏻

if err != nil {
return nil, nil, errors.Wrap(err, errParserPackage)
}
Expand All @@ -167,6 +192,43 @@ func (b *Builder) Build(ctx context.Context, opts ...BuildOpt) (v1.Image, runtim
if meta.GetObjectKind().GroupVersionKind().Kind == pkgmetav1.ConfigurationKind {
linter = NewConfigurationLinter()
} else {
if b.ab != nil { // if we have an auth.yaml file
if p, ok := meta.(*v1alpha1.Provider); ok {
nullable-eth marked this conversation as resolved.
Show resolved Hide resolved
// if has annotation auth.upbound.io/group then look for the object
// specified there like aws.upbound.io and annotate that with auth.upbound.io/config
// and embed the contents of the auth.yaml file
if group, ok := p.ObjectMeta.Annotations[authMetaAnno]; ok {
ar, err := b.ab.Init(ctx)
nullable-eth marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, nil, errors.Wrap(err, errParseAuth)
}

// validate the auth.yaml file
var auth AuthExtension
if err := yaml.NewDecoder(ar).Decode(&auth); err != nil {
return nil, nil, errors.Wrap(err, errParseAuth)
}
annotated := false
for x, o := range pkg.GetObjects() {
if c, ok := o.(*crd.CustomResourceDefinition); ok {
if c.Spec.Group == group && c.Spec.Names.Kind == ProviderConfigKind {
ab := new(bytes.Buffer)
if err := yaml.NewEncoder(ab).Encode(auth); err != nil {
return nil, nil, errors.Wrap(err, errParseAuth)
}
c.Annotations[authObjectAnno] = ab.String()
pkg.GetObjects()[x] = c
annotated = true
nullable-eth marked this conversation as resolved.
Show resolved Hide resolved
break
}
}
}
if !annotated {
return nil, nil, errors.New(errAuthNotAnnotated)
}
}
}
}
linter = NewProviderLinter()
}
if err := linter.Lint(pkg); err != nil {
Expand All @@ -182,7 +244,12 @@ func (b *Builder) Build(ctx context.Context, opts ...BuildOpt) (v1.Image, runtim
cfg := cfgFile.Config
cfg.Labels = make(map[string]string)

pkgLayer, err := Layer(pkgBuf, StreamFile, PackageAnnotation, int64(pkgBuf.Len()), &cfg)
pkgBytes, err := encode(pkg)
if err != nil {
return nil, nil, errors.Wrap(err, errConfigFile)
}

pkgLayer, err := Layer(pkgBytes, StreamFile, PackageAnnotation, int64(pkgBytes.Len()), &cfg)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -217,6 +284,30 @@ func (b *Builder) Build(ctx context.Context, opts ...BuildOpt) (v1.Image, runtim
return bOpts.base, meta, nil
}

// encode encodes a package as a YAML stream. Does not check meta existence
// or quantity i.e. it should be linted first to ensure that it is valid.
func encode(pkg linter.Package) (*bytes.Buffer, error) {
pkgBuf := new(bytes.Buffer)
objScheme, err := scheme.BuildObjectScheme()
if err != nil {
return nil, errors.New(errBuildObjectScheme)
}

do := json.NewSerializerWithOptions(json.DefaultMetaFactory, objScheme, objScheme, json.SerializerOptions{Yaml: true})
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm does this serializer work with the meta object? I was thinking we would just serializing without schemes altogether here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, in fact in encoding it doesn't really use the object schemes at all https://github.com/kubernetes/apimachinery/blob/8d1258da8f386b809d312cdda316366d5612f54e/pkg/runtime/serializer/json/json.go#L223 but in the interest of using k8s machinery and dealing with runtime objects it seemed to more streamlined option than using a straight yaml serializer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! 👍🏻

pkgBuf.WriteString("---\n")
if err = do.Encode(pkg.GetMeta()[0], pkgBuf); err != nil {
return nil, errors.Wrap(err, errBuildObjectScheme)
}
pkgBuf.WriteString("---\n")
for _, o := range pkg.GetObjects() {
if err = do.Encode(o, pkgBuf); err != nil {
return nil, errors.Wrap(err, errBuildObjectScheme)
}
pkgBuf.WriteString("---\n")
nullable-eth marked this conversation as resolved.
Show resolved Hide resolved
}
return pkgBuf, nil
}

// SkipContains supplies a FilterFn that skips paths that contain the give pattern.
func SkipContains(pattern string) parser.FilterFn {
return func(path string, info os.FileInfo) (bool, error) {
Expand Down
4 changes: 2 additions & 2 deletions internal/xpkg/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestBuild(t *testing.T) {

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
builder := New(tc.args.be, tc.args.ex, tc.args.p, tc.args.e)
builder := New(tc.args.be, nil, tc.args.ex, tc.args.p, tc.args.e)

_, _, err := builder.Build(context.TODO())

Expand Down Expand Up @@ -251,7 +251,7 @@ func TestBuildExamples(t *testing.T) {
parser.FsFilters(defaultFilters...),
)

builder := New(pkgBe, pkgEx, pkgp, examples.New())
builder := New(pkgBe, nil, pkgEx, pkgp, examples.New())

img, _, err := builder.Build(context.TODO())

Expand Down