Skip to content

Commit

Permalink
Download command should look for image based on the commit (#48)
Browse files Browse the repository at this point in the history
* Rather than downloading assets based on the latest tag we should pick
the image whose commit matches the commit of the binary

* This ensures the frontend and backend are built off the same version

* If the commit isn't specified in the build (E.g. a development
version) then we default to the latest tag

Related to #45
  • Loading branch information
jlewi authored Apr 11, 2024
1 parent 91ddad2 commit bcdad7b
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 19 deletions.
21 changes: 20 additions & 1 deletion app/cmd/assets.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,19 @@ import (
"fmt"
"os"

"github.com/go-logr/zapr"
"go.uber.org/zap"

"github.com/jlewi/foyle/app/pkg/application"
"github.com/jlewi/foyle/app/pkg/assets"
"github.com/jlewi/monogo/helpers"
"github.com/spf13/cobra"
)

const (
defaultTag = "latest"
)

// NewAssetsCmd returns a command to download the assets
func NewAssetsCmd() *cobra.Command {
cmd := &cobra.Command{
Expand All @@ -23,6 +30,7 @@ func NewAssetsCmd() *cobra.Command {

// NewAssetsDownloadCmd returns a command to download the assets
func NewAssetsDownloadCmd() *cobra.Command {
var tag string
cmd := &cobra.Command{
Use: "download",
Run: func(cmd *cobra.Command, args []string) {
Expand All @@ -43,7 +51,17 @@ func NewAssetsDownloadCmd() *cobra.Command {
return err
}

if err := m.Download(context.Background()); err != nil {
if tag == "" {
if commit == commitNotSet {
// Since the commit isn't set we are using a development build so we use the latest tag
tag = defaultTag
} else {
tag = commit
}
}
log := zapr.NewLogger(zap.L())
log.Info("Downloading assets", "tag", tag)
if err := m.Download(context.Background(), tag); err != nil {
return err
}
return nil
Expand All @@ -56,5 +74,6 @@ func NewAssetsDownloadCmd() *cobra.Command {
},
}

cmd.Flags().StringVarP(&tag, "tag", "", "", "The tag for the assets to download. If empty downloads the assets matching the commit of the binary")
return cmd
}
6 changes: 5 additions & 1 deletion app/cmd/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,16 @@ import (
"github.com/spf13/cobra"
)

const (
commitNotSet = "none"
)

// These constants will be set by ldflags.
// They can be set by goreleaser
// https://goreleaser.com/cookbooks/using-main.version/?h=using+main.version
var (
version = "dev"
commit = "none"
commit = commitNotSet
date = "unknown"
builtBy = "unknown"
)
Expand Down
38 changes: 34 additions & 4 deletions app/pkg/assets/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"path/filepath"

"github.com/go-logr/zapr"
"github.com/google/go-containerregistry/pkg/name"
"github.com/jlewi/foyle/app/pkg/config"
"github.com/jlewi/foyle/app/pkg/logs"
"github.com/jlewi/hydros/pkg/files"
Expand All @@ -21,6 +22,9 @@ const (
// Constants representing subdirectories for different assets
vscode = "vscode"
extension = "foyle"

defaultVSCodeImage = "ghcr.io/jlewi/vscode-web-assets"
defaultFoyleImage = "ghcr.io/jlewi/foyle-vscode-ext"
)

// Manager is a struct that manages assets
Expand All @@ -42,20 +46,31 @@ type asset struct {
}

// Download downloads the assets
func (m *Manager) Download(ctx context.Context) error {
func (m *Manager) Download(ctx context.Context, tag string) error {
log := logs.FromContext(ctx)

// Map from the name of the asset to the source of the location
assets := map[string]asset{
assets := map[string]*asset{
vscode: {
source: m.config.Assets.VSCode.URI,
source: defaultVSCodeImage,
stripPrefix: "assets",
},
extension: {
source: m.config.Assets.FoyleExtension.URI,
source: defaultFoyleImage,
stripPrefix: "foyle",
},
}

// If any assets are specified in the config file then the should override the defaults
if m.config.Assets != nil {
if m.config.Assets.VSCode != nil && m.config.Assets.VSCode.URI != "" {
assets[vscode].source = m.config.Assets.VSCode.URI
}
if m.config.Assets.FoyleExtension != nil && m.config.Assets.FoyleExtension.URI != "" {
assets[extension].source = m.config.Assets.FoyleExtension.URI
}
}

if m.downloadDir == "" {
tDir, err := os.MkdirTemp("", "temporaryAssets")
if err != nil {
Expand Down Expand Up @@ -90,6 +105,12 @@ func (m *Manager) Download(ctx context.Context) error {
if uri == "" {
return errors.Errorf("Asset %s has an empty source", name)
}

uri, err = resolveTag(uri, tag)
if err != nil {
return errors.Wrapf(err, "Error resolving tag for asset %v", name)
}

// TODO(jeremy): Should we check if its an empty directory
if _, err := os.Stat(assetDir); err == nil {
log.Info("Asset already exists", "assetDir", assetDir, "name", name, "source", source)
Expand Down Expand Up @@ -189,3 +210,12 @@ func unpackTarball(srcTarball string, dest string, stripPrefix string) error {
}
}
}

// resolveTag checks if repo has a tag and if not it adds the tag specified by tag
func resolveTag(repo string, tag string) (string, error) {
ref, err := name.ParseReference(repo, name.WithDefaultTag(tag))
if err != nil {
return "", errors.Wrapf(err, "Error parsing reference %v", repo)
}
return ref.Name(), nil
}
36 changes: 35 additions & 1 deletion app/pkg/assets/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,47 @@ package assets

import (
"context"
"fmt"
"os"
"testing"

"github.com/jlewi/foyle/app/pkg/config"
"go.uber.org/zap"
)

func Test_ResolveImage(t *testing.T) {
type testCase struct {
image string
tag string
expected string
}

cases := []testCase{
{
image: "ghcr.io/jlewi/vscode-web-assets",
tag: "1234",
expected: "ghcr.io/jlewi/vscode-web-assets:1234",
},
{
image: "ghcr.io/jlewi/vscode-web-assets:abcd",
tag: "1234",
expected: "ghcr.io/jlewi/vscode-web-assets:abcd",
},
}

for i, c := range cases {
t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) {
actual, err := resolveTag(c.image, c.tag)
if err != nil {
t.Fatalf("Error resolving tag; %v", err)
}
if actual != c.expected {
t.Errorf("Expected %v; got %v", c.expected, actual)
}
})
}
}

func Test_Download(t *testing.T) {
if os.Getenv("GITHUB_ACTIONS") != "" {
t.Skipf("Test is skipped in GitHub actions")
Expand All @@ -35,7 +69,7 @@ func Test_Download(t *testing.T) {
t.Fatalf("Error creating manager; %v", err)
}

if err := m.Download(context.Background()); err != nil {
if err := m.Download(context.Background(), "latest"); err != nil {
t.Fatalf("Error downloading assets; %v", err)
}
}
15 changes: 3 additions & 12 deletions app/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ const (
LevelFlagName = "level"
appName = "foyle"
ConfigDir = "." + appName

defaultVSCodeImage = "ghcr.io/jlewi/vscode-web-assets:latest"
defaultFoyleImage = "ghcr.io/jlewi/foyle-vscode-ext:latest"
)

// Config represents the persistent configuration data for Foyle.
Expand All @@ -40,7 +37,7 @@ type Config struct {

Logging Logging `json:"logging" yaml:"logging"`
Server ServerConfig `json:"server" yaml:"server"`
Assets AssetConfig `json:"assets" yaml:"assets"`
Assets *AssetConfig `json:"assets,omitempty" yaml:"assets,omitempty"`
Agent *AgentConfig `json:"agent,omitempty" yaml:"agent,omitempty"`
OpenAI *OpenAIConfig `json:"openai,omitempty" yaml:"openai,omitempty"`
// AzureOpenAI contains configuration for Azure OpenAI. A non nil value means use Azure OpenAI.
Expand Down Expand Up @@ -124,8 +121,8 @@ type CorsConfig struct {

// AssetConfig configures the assets
type AssetConfig struct {
VSCode Asset `json:"vsCode" yaml:"vsCode"`
FoyleExtension Asset `json:"foyleExtension" yaml:"foyleExtension"`
VSCode *Asset `json:"vsCode,omitempty" yaml:"vsCode,omitempty"`
FoyleExtension *Asset `json:"foyleExtension,omitempty" yaml:"foyleExtension,omitempty"`
}

type Asset struct {
Expand Down Expand Up @@ -185,7 +182,6 @@ func InitViper(cmd *cobra.Command) error {

setAgentDefaults()
setServerDefaults()
setAssetDefaults()

// We need to attach to the command line flag if it was specified.
keyToflagName := map[string]string{
Expand Down Expand Up @@ -286,11 +282,6 @@ func setServerDefaults() {
viper.SetDefault("server.httpMaxReadTimeout", 1*time.Minute)
}

func setAssetDefaults() {
viper.SetDefault("assets.vsCode.uri", defaultVSCodeImage)
viper.SetDefault("assets.foyleExtension.uri", defaultFoyleImage)
}

func setAgentDefaults() {
viper.SetDefault("agent.model", DefaultModel)
}
Expand Down

0 comments on commit bcdad7b

Please sign in to comment.