Skip to content

Commit

Permalink
fix: check if image extended after serialisation (#2329)
Browse files Browse the repository at this point in the history
* fix: check if image extended after serialisation

Signed-off-by: Javier López Barba <javier@okteto.com>

* fix: unit tests

Signed-off-by: Javier López Barba <javier@okteto.com>

* fix: show warning once instead of once per dev

Signed-off-by: Javier López Barba <javier@okteto.com>

* fix: windows unit tests

Signed-off-by: Javier López Barba <javier@okteto.com>
  • Loading branch information
jLopezbarb committed Mar 8, 2022
1 parent 3b775e8 commit 666b87e
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 75 deletions.
2 changes: 1 addition & 1 deletion cmd/utils/dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func Test_LoadManifestOrDefault(t *testing.T) {
expectErr: false,
dev: &model.Dev{
Name: "loaded",
Image: &model.DevBuildInfo{
Image: &model.BuildInfo{
Name: "okteto/test:1.0",
},
Sync: model.Sync{
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/doctor/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ func generateManifestFile(devPath string) (string, error) {
}

dev := &model.Dev{
Image: &model.DevBuildInfo{},
Push: &model.DevBuildInfo{},
Image: &model.BuildInfo{},
Push: &model.BuildInfo{},
Environment: make([]model.EnvVar, 0),
Secrets: make([]model.Secret, 0),
Forward: make([]model.Forward, 0),
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/doctor/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,19 @@ func Test_generateManifestFile(t *testing.T) {
name: "basic",
dev: &model.Dev{
Name: "dev",
Image: &model.DevBuildInfo{Name: "okteto/dev"},
Image: &model.BuildInfo{Name: "okteto/dev"},
Command: model.Command{Values: []string{"bash"}},
},
},
{
name: "with-services",
dev: &model.Dev{
Name: "dev",
Image: &model.DevBuildInfo{Name: "okteto/dev"},
Image: &model.BuildInfo{Name: "okteto/dev"},
Command: model.Command{Values: []string{"bash"}},
Services: []*model.Dev{{
Name: "svc",
Image: &model.DevBuildInfo{Name: "okteto/svc"},
Image: &model.BuildInfo{Name: "okteto/svc"},
Command: model.Command{Values: []string{"bash"}},
}, {
Name: "svc2",
Expand Down
4 changes: 2 additions & 2 deletions pkg/k8s/apps/crud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestGetStatefulset(t *testing.T) {
dev := &model.Dev{
Name: "test",
Namespace: "test",
Image: &model.DevBuildInfo{
Image: &model.BuildInfo{
Name: "image",
},
PersistentVolumeInfo: &model.PersistentVolumeInfo{
Expand Down Expand Up @@ -114,7 +114,7 @@ func TestGetDeployment(t *testing.T) {
dev := &model.Dev{
Name: "test",
Namespace: "test",
Image: &model.DevBuildInfo{
Image: &model.BuildInfo{
Name: "image",
},
PersistentVolumeInfo: &model.PersistentVolumeInfo{
Expand Down
2 changes: 1 addition & 1 deletion pkg/linguist/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func GetDevDefaults(language, workdir string) (*model.Dev, error) {
vals := languageDefaults[language]

dev := &model.Dev{
Image: &model.DevBuildInfo{
Image: &model.BuildInfo{
Name: vals.image,
},
Command: model.Command{
Expand Down
25 changes: 5 additions & 20 deletions pkg/model/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ type Dev struct {
Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"`
Container string `json:"container,omitempty" yaml:"container,omitempty"`
EmptyImage bool `json:"-" yaml:"-"`
Image *DevBuildInfo `json:"image,omitempty" yaml:"image,omitempty"`
Push *DevBuildInfo `json:"-" yaml:"push,omitempty"`
Image *BuildInfo `json:"image,omitempty" yaml:"image,omitempty"`
Push *BuildInfo `json:"-" yaml:"push,omitempty"`
ImagePullPolicy apiv1.PullPolicy `json:"imagePullPolicy,omitempty" yaml:"imagePullPolicy,omitempty"`
Secrets []Secret `json:"secrets,omitempty" yaml:"secrets,omitempty"`
Command Command `json:"command,omitempty" yaml:"command,omitempty"`
Expand Down Expand Up @@ -126,9 +126,6 @@ type BuildInfo struct {
VolumesToInclude []StackVolume `yaml:"-"`
}

// DevBuildInfo throws a warning if it's not single line
type DevBuildInfo BuildInfo

// Volume represents a volume in the development container
type Volume struct {
LocalPath string
Expand Down Expand Up @@ -302,8 +299,8 @@ func Get(devPath string) (*Manifest, error) {
}
func NewDev() *Dev {
return &Dev{
Image: &DevBuildInfo{},
Push: &DevBuildInfo{},
Image: &BuildInfo{},
Push: &BuildInfo{},
Environment: make(Environment, 0),
Secrets: make([]Secret, 0),
Forward: make([]Forward, 0),
Expand Down Expand Up @@ -428,7 +425,7 @@ func (dev *Dev) loadSelector() error {
func (dev *Dev) loadImage() error {
var err error
if dev.Image == nil {
dev.Image = &DevBuildInfo{}
dev.Image = &BuildInfo{}
}
if len(dev.Image.Name) > 0 {
dev.Image.Name, err = ExpandEnv(dev.Image.Name)
Expand Down Expand Up @@ -551,18 +548,6 @@ func (dev *Dev) SetDefaults() error {
return nil
}

func (build *DevBuildInfo) setBuildDefaults() {
if build == nil {
build = &DevBuildInfo{}
}
if build.Context == "" {
build.Context = "."
}
if _, err := url.ParseRequestURI(build.Context); err != nil && build.Dockerfile == "" {
build.Dockerfile = "Dockerfile"
}
}

func (build *BuildInfo) setBuildDefaults() {
if build == nil {
build = &BuildInfo{}
Expand Down
10 changes: 5 additions & 5 deletions pkg/model/dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,8 @@ func TestDev_validateName(t *testing.T) {
dev := &Dev{
Name: tt.devName,
ImagePullPolicy: apiv1.PullAlways,
Image: &DevBuildInfo{},
Push: &DevBuildInfo{},
Image: &BuildInfo{},
Push: &BuildInfo{},
Sync: Sync{
Folders: []SyncFolder{
{
Expand All @@ -533,15 +533,15 @@ func TestDev_readImageContext(t *testing.T) {
tests := []struct {
name string
manifest []byte
expected *DevBuildInfo
expected *BuildInfo
}{
{
name: "context pointing to url",
manifest: []byte(`name: deployment
image:
context: https://github.com/okteto/okteto.git
`),
expected: &DevBuildInfo{
expected: &BuildInfo{
Context: "https://github.com/okteto/okteto.git",
},
},
Expand All @@ -551,7 +551,7 @@ image:
image:
context: .
`),
expected: &DevBuildInfo{
expected: &BuildInfo{
Context: ".",
Dockerfile: "Dockerfile",
},
Expand Down
8 changes: 8 additions & 0 deletions pkg/model/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,14 @@ func Read(bytes []byte) (*Manifest, error) {
return nil, errors.New(msg)
}
}
hasShownWarning := false
for _, d := range manifest.Dev {
if (d.Image.Context != "" || d.Image.Dockerfile != "") && !hasShownWarning {
hasShownWarning = true
oktetoLog.Yellow(`The 'image' extended syntax is deprecated. Define the images you want to build in the 'build' section of your manifest. More info at https://www.okteto.com/docs/reference/manifest/#build"`)
}
}

if err := manifest.setDefaults(); err != nil {
return nil, err
}
Expand Down
17 changes: 11 additions & 6 deletions pkg/model/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package model

import (
"os"
"runtime"
"testing"
"time"

Expand Down Expand Up @@ -83,6 +84,10 @@ devs:
}

func TestInferFromStack(t *testing.T) {
devInterface := PrivilegedLocalhost
if runtime.GOOS == "windows" {
devInterface = Localhost
}
stack := &Stack{
Services: map[string]*Service{
"test": {
Expand Down Expand Up @@ -139,11 +144,11 @@ func TestInferFromStack(t *testing.T) {
},
},
EmptyImage: true,
Image: &DevBuildInfo{
Image: &BuildInfo{
Context: ".",
Dockerfile: "Dockerfile",
},
Push: &DevBuildInfo{
Push: &BuildInfo{
Context: ".",
Dockerfile: "Dockerfile",
},
Expand Down Expand Up @@ -172,7 +177,7 @@ func TestInferFromStack(t *testing.T) {
Command: Command{
Values: []string{"sh"},
},
Interface: "0.0.0.0",
Interface: devInterface,
Services: []*Dev{},
Sync: Sync{
RescanInterval: 300,
Expand Down Expand Up @@ -230,11 +235,11 @@ func TestInferFromStack(t *testing.T) {
},
},
EmptyImage: true,
Image: &DevBuildInfo{
Image: &BuildInfo{
Context: ".",
Dockerfile: "Dockerfile",
},
Push: &DevBuildInfo{
Push: &BuildInfo{
Context: ".",
Dockerfile: "Dockerfile",
},
Expand Down Expand Up @@ -317,7 +322,7 @@ func TestInferFromStack(t *testing.T) {
},
Selector: Selector{},
EmptyImage: true,
Image: &DevBuildInfo{
Image: &BuildInfo{
Context: ".",
Dockerfile: "Dockerfile",
},
Expand Down
18 changes: 0 additions & 18 deletions pkg/model/serializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,24 +286,6 @@ func (sync Sync) MarshalYAML() (interface{}, error) {
return syncRaw(sync), nil
}

// UnmarshalYAML Implements the Unmarshaler interface of the yaml pkg.
func (buildInfo *DevBuildInfo) UnmarshalYAML(unmarshal func(interface{}) error) error {
var rawString string
err := unmarshal(&rawString)
if err == nil {
buildInfo.Name = rawString
return nil
}
var rawBuildInfo BuildInfo
err = unmarshal(&rawBuildInfo)
if err != nil {
return err
}
oktetoLog.Yellow("The `image` extended syntax is deprecated. Define the images you want to build in the 'build' section of your manifest. More info at https://www.okteto.com/docs/reference/manifest/#build")
*buildInfo = DevBuildInfo(rawBuildInfo)
return nil
}

// UnmarshalYAML Implements the Unmarshaler interface of the yaml pkg.
func (buildInfo *BuildInfo) UnmarshalYAML(unmarshal func(interface{}) error) error {
var rawString string
Expand Down

0 comments on commit 666b87e

Please sign in to comment.