diff --git a/functions/go/set-image/custom/custom.go b/functions/go/set-image/custom/custom.go index 2b200262b..adcaf59ee 100644 --- a/functions/go/set-image/custom/custom.go +++ b/functions/go/set-image/custom/custom.go @@ -31,7 +31,7 @@ func SetAdditionalFieldSpec(img *fn.SubObject, objects fn.KubeObjects, addImgFie if err != nil { ctx.ResultErr(err.Error(), obj) } - objects[i] = newObj + *objects[i] = *newObj } } diff --git a/functions/go/set-image/transformer/images_transformer.go b/functions/go/set-image/transformer/images_transformer.go index e20dc328c..feef8c029 100644 --- a/functions/go/set-image/transformer/images_transformer.go +++ b/functions/go/set-image/transformer/images_transformer.go @@ -2,19 +2,24 @@ package transformer import ( "fmt" - "github.com/GoogleContainerTools/kpt-functions-catalog/functions/go/set-image/custom" "github.com/GoogleContainerTools/kpt-functions-catalog/functions/go/set-image/third_party/sigs.k8s.io/kustomize/api/image" "github.com/GoogleContainerTools/kpt-functions-catalog/functions/go/set-image/third_party/sigs.k8s.io/kustomize/api/types" "github.com/GoogleContainerTools/kpt-functions-sdk/go/fn" ) -// Image contains an image name, a new name, a new tag or digest, which will replace the original name and tag. type Image struct { - // Name is a tag-less image name. + // DEPRECATED + //Name is a tag-less image name. Name string `json:"name,omitempty" yaml:"name,omitempty"` - // NewName is the value used to replace the original name. + // ContainerName is the Pod's container name. It is used to choose the matching containers to update the images. + ContainerName string `json:"containerName,omitempty" yaml:"containerName,omitempty"` + + // ImageName is the image name. It is used to choose images for update. + ImageName string `json:"imageName,omitempty" yaml:"imageName,omitempty"` + + // NewName is the new image name NewName string `json:"newName,omitempty" yaml:"newName,omitempty"` // NewTag is the value used to replace the original tag. @@ -43,21 +48,28 @@ func (t SetImage) Run(ctx *fn.Context, functionConfig *fn.KubeObject, items fn.K if err != nil { ctx.ResultErrAndDie(err.Error(), nil) } - err = t.validateInput() - if err != nil { - ctx.ResultErrAndDie(err.Error(), nil) + + items = items.WhereNot(fn.IsLocalConfig) + + res := t.validateInput(items) + if res != nil { + if res.Severity == fn.Error { + ctx.ResultErrAndDie(res.Message, nil) + } + ctx.Result(res.Message, res.Severity) + return } for _, o := range items { - switch o.GetKind() { - case "Pod": - if err = t.setPodContainers(o); err != nil { - ctx.ResultErr(err.Error(), o) - } - case "Deployment", "StatefulSet", "ReplicaSet", "DaemonSet", "PodTemplate": - if err = t.setPodSpecContainers(o); err != nil { - ctx.ResultErr(err.Error(), o) - } + containers := getContainers(o) + warnings, err := t.updateContainerImages(containers) + + for _, w := range warnings { + ctx.ResultWarn(w.Message, o) + } + + if err != nil { + ctx.ResultErr(err.Error(), o) } } @@ -74,7 +86,11 @@ func (t *SetImage) configDefaultData() error { for key, val := range t.DataFromDefaultConfig { switch key { case "name": - t.Image.Name = val + t.Image.ImageName = val + case "containerName": + t.Image.ContainerName = val + case "imageName": + t.Image.ImageName = val case "newName": t.Image.NewName = val case "newTag": @@ -89,68 +105,144 @@ func (t *SetImage) configDefaultData() error { } // validateInput validates the inputs passed into via the functionConfig -func (t *SetImage) validateInput() error { - // TODO: support container name and only one argument input in the next PR - if t.Image.Name == "" { - return fmt.Errorf("must specify `name`") +func (t *SetImage) validateInput(items fn.KubeObjects) *fn.Result { + // if user does not input image name or container name, there should be only one image name to select from + if t.Image.Name == "" && t.Image.ImageName == "" && t.Image.ContainerName == "" { + if !t.isImageNameUnique(items) { + return fn.GeneralResult("must specify `imageName`, resources contain non-unique image names", fn.Error) + } + } + if t.Image.Name != "" && t.Image.ImageName != "" && t.Image.Name != t.Image.ImageName { + return fn.GeneralResult("must not fill `imageName` and `name` at same time, their values should be equal", fn.Error) } if t.Image.NewName == "" && t.Image.NewTag == "" && t.Image.Digest == "" { - return fmt.Errorf("must specify one of `newName`, `newTag`, or `digest`") + return fn.GeneralResult("must specify one of `newName`, `newTag`, or `digest`", fn.Error) + } + if len(items) == 0 { + return fn.GeneralResult("no input resources", fn.Info) } return nil } -// updateContainerImages updates the images inside containers, return potential error -func (t *SetImage) updateContainerImages(pod *fn.SubObject) error { - var containers fn.SliceSubObjects - containers = append(containers, pod.GetSlice("iniContainers")...) - containers = append(containers, pod.GetSlice("containers")...) +// matchImage takes the resources image name and container name, return if there is a match and potential warning +func matchImage(oldImageName string, oldContainer string, newImage *Image) (bool, *fn.Result) { + // name would be deprecated, means image name + if newImage.Name != "" { + newImage.ImageName = newImage.Name + } + + // match image name, no container name + if newImage.ImageName != "" && newImage.ContainerName == "" { + if !image.IsImageMatched(oldImageName, newImage.ImageName) { + return false, nil + } + } + // match container name, no image name + if newImage.ImageName == "" && newImage.ContainerName != "" { + if oldContainer != newImage.ContainerName { + return false, nil + } + } + // match both + if newImage.ImageName != "" && newImage.ContainerName != "" { + if !image.IsImageMatched(oldImageName, newImage.ImageName) { + return false, nil + } + //if only image name match, container name does not match, provide a warning + if oldContainer != newImage.ContainerName { + msg := fmt.Sprintf("container name `%v` does not match `%v`, only image name matches", newImage.ContainerName, oldContainer) + warning := &fn.Result{ + Message: msg, + Severity: fn.Warning, + } + return true, warning + } + } + return true, nil +} + +// isImageNameUnique checks if there is only one image name in all resources, return true if name is unique +func (t *SetImage) isImageNameUnique(items fn.KubeObjects) bool { + curImageName := "" + for _, o := range items { + containers := getContainers(o) + for _, c := range containers { + oldValue := c.NestedStringOrDie("image") + imageName, _, _ := image.Split(oldValue) + if curImageName == "" { + curImageName = imageName + } else if curImageName != imageName { + return false + } + } + } + return true +} +// updateContainerImages updates the images inside containers, return warnings and error +func (t *SetImage) updateContainerImages(containers fn.SliceSubObjects) (fn.Results, error) { + var warningList fn.Results for _, o := range containers { oldValue := o.NestedStringOrDie("image") - if !image.IsImageMatched(oldValue, t.Image.Name) { + oldContainer := o.NestedStringOrDie("name") + + matched, warning := matchImage(oldValue, oldContainer, &t.Image) + if !matched { continue } + newName := getNewImageName(oldValue, t.Image) if oldValue == newName { continue } + if warning != nil { + warningList = append(warningList, warning) + } + if err := o.SetNestedString(newName, "image"); err != nil { - return err + return warningList, err } t.resultCount += 1 } + return warningList, nil +} + +// getContainers gets the containers inside kubeObject +func getContainers(o *fn.KubeObject) fn.SliceSubObjects { + switch o.GetKind() { + case "Pod": + return getPodContainers(o) + case "Deployment", "StatefulSet", "ReplicaSet", "DaemonSet", "PodTemplate": + return getPodSpecContainers(o) + } return nil } -func (t *SetImage) setPodSpecContainers(o *fn.KubeObject) error { +// getPodContainers gets the containers from pod +func getPodContainers(o *fn.KubeObject) fn.SliceSubObjects { spec := o.GetMap("spec") if spec == nil { return nil } - template := spec.GetMap("template") - if template == nil { - return nil - } - podSpec := template.GetMap("spec") - err := t.updateContainerImages(podSpec) - if err != nil { - return err - } - return nil + return append(spec.GetSlice("iniContainers"), spec.GetSlice("containers")...) } -func (t *SetImage) setPodContainers(o *fn.KubeObject) error { +// getPodSpecContainers gets the containers from podSpec +func getPodSpecContainers(o *fn.KubeObject) fn.SliceSubObjects { spec := o.GetMap("spec") if spec == nil { return nil } - err := t.updateContainerImages(spec) - if err != nil { - return err + template := spec.GetMap("template") + if template == nil { + return nil } - return nil + podSpec := template.GetMap("spec") + if podSpec == nil { + return nil + } + return append(podSpec.GetSlice("iniContainers"), podSpec.GetSlice("containers")...) } // getNewImageName return the new name for image field diff --git a/tests/set-image/containerName-mismatch/.expected/diff.patch b/tests/set-image/containerName-mismatch/.expected/diff.patch new file mode 100644 index 000000000..909a42896 --- /dev/null +++ b/tests/set-image/containerName-mismatch/.expected/diff.patch @@ -0,0 +1,14 @@ +diff --git a/resources.yaml b/resources.yaml +index f674e0b..ae7d829 100644 +--- a/resources.yaml ++++ b/resources.yaml +@@ -5,6 +5,6 @@ metadata: + spec: + containers: + - name: server +- image: nginx:1.20.2 ++ image: bar:v1.0 + - name: store +- image: nginx:14.1 +\ No newline at end of file ++ image: bar:v1.0 \ No newline at end of file diff --git a/tests/set-image/containerName-mismatch/.expected/results.yaml b/tests/set-image/containerName-mismatch/.expected/results.yaml new file mode 100644 index 000000000..95c1b3fdf --- /dev/null +++ b/tests/set-image/containerName-mismatch/.expected/results.yaml @@ -0,0 +1,19 @@ +apiVersion: kpt.dev/v1 +kind: FunctionResultList +metadata: + name: fnresults +exitCode: 0 +items: + - image: gcr.io/kpt-fn/set-image:unstable + exitCode: 0 + results: + - message: container name `server` does not match `store`, only image name matches + severity: warning + resourceRef: + apiVersion: v1 + kind: Pod + name: app1 + file: + path: resources.yaml + - message: 'summary: updated a total of 2 image(s)' + severity: info \ No newline at end of file diff --git a/tests/set-image/no-image-name/.krmignore b/tests/set-image/containerName-mismatch/.krmignore similarity index 100% rename from tests/set-image/no-image-name/.krmignore rename to tests/set-image/containerName-mismatch/.krmignore diff --git a/tests/set-image/containerName-mismatch/Kptfile b/tests/set-image/containerName-mismatch/Kptfile new file mode 100644 index 000000000..8dc7cb5f3 --- /dev/null +++ b/tests/set-image/containerName-mismatch/Kptfile @@ -0,0 +1,12 @@ +apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: example +pipeline: + mutators: + - image: gcr.io/kpt-fn/set-image:unstable + configMap: + imageName: nginx + containerName: server + newName: bar + newTag: v1.0 diff --git a/tests/set-image/containerName-mismatch/resources.yaml b/tests/set-image/containerName-mismatch/resources.yaml new file mode 100644 index 000000000..f674e0b7a --- /dev/null +++ b/tests/set-image/containerName-mismatch/resources.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: Pod +metadata: + name: app1 +spec: + containers: + - name: server + image: nginx:1.20.2 + - name: store + image: nginx:14.1 \ No newline at end of file diff --git a/tests/set-image/empty-resource/.expected/results.yaml b/tests/set-image/empty-resource/.expected/results.yaml new file mode 100644 index 000000000..5cc084f2f --- /dev/null +++ b/tests/set-image/empty-resource/.expected/results.yaml @@ -0,0 +1,11 @@ +apiVersion: kpt.dev/v1 +kind: FunctionResultList +metadata: + name: fnresults +exitCode: 0 +items: + - image: gcr.io/kpt-fn/set-image:unstable + exitCode: 0 + results: + - message: no input resources + severity: info \ No newline at end of file diff --git a/tests/set-image/empty-resource/.krmignore b/tests/set-image/empty-resource/.krmignore new file mode 100644 index 000000000..9d7a4007d --- /dev/null +++ b/tests/set-image/empty-resource/.krmignore @@ -0,0 +1 @@ +.expected diff --git a/tests/set-image/empty-resource/Kptfile b/tests/set-image/empty-resource/Kptfile new file mode 100644 index 000000000..7e066551a --- /dev/null +++ b/tests/set-image/empty-resource/Kptfile @@ -0,0 +1,14 @@ +apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: example + annotations: + config.kubernetes.io/local-config: "true" +pipeline: + mutators: + - image: gcr.io/kpt-fn/set-image:unstable + configMap: + imageName: nginx + containerName: server + newName: bar + newTag: v1.0 diff --git a/tests/set-image/empty-resource/resources.yaml b/tests/set-image/empty-resource/resources.yaml new file mode 100644 index 000000000..e69de29bb diff --git a/tests/set-image/no-image-name/resources.yaml b/tests/set-image/no-image-name/resources.yaml deleted file mode 100644 index ee6f8a24b..000000000 --- a/tests/set-image/no-image-name/resources.yaml +++ /dev/null @@ -1,7 +0,0 @@ -apiVersion: config.kubernetes.io/v1 -kind: ResourceList -metadata: - name: my-resource -items: -- apiVersion: v1 - kind: ConfigMap diff --git a/tests/set-image/no-image-name/.expected/config.yaml b/tests/set-image/nonunique-image-with-no-image-name-input/.expected/config.yaml similarity index 100% rename from tests/set-image/no-image-name/.expected/config.yaml rename to tests/set-image/nonunique-image-with-no-image-name-input/.expected/config.yaml diff --git a/tests/set-image/no-image-name/.expected/results.yaml b/tests/set-image/nonunique-image-with-no-image-name-input/.expected/results.yaml similarity index 51% rename from tests/set-image/no-image-name/.expected/results.yaml rename to tests/set-image/nonunique-image-with-no-image-name-input/.expected/results.yaml index eca13cd4e..8082a52ce 100644 --- a/tests/set-image/no-image-name/.expected/results.yaml +++ b/tests/set-image/nonunique-image-with-no-image-name-input/.expected/results.yaml @@ -5,10 +5,10 @@ metadata: exitCode: 1 items: - image: gcr.io/kpt-fn/set-image:unstable - stderr: 'failed to evaluate function: function is terminated: must specify `name`' + stderr: 'failed to evaluate function: function is terminated: must specify `imageName`, resources contain non-unique image names' exitCode: 1 results: - - message: must specify `name` + - message: must specify `imageName`, resources contain non-unique image names severity: error - - message: 'function is terminated: must specify `name`' + - message: 'function is terminated: must specify `imageName`, resources contain non-unique image names' severity: error \ No newline at end of file diff --git a/tests/set-image/nonunique-image-with-no-image-name-input/.krmignore b/tests/set-image/nonunique-image-with-no-image-name-input/.krmignore new file mode 100644 index 000000000..9d7a4007d --- /dev/null +++ b/tests/set-image/nonunique-image-with-no-image-name-input/.krmignore @@ -0,0 +1 @@ +.expected diff --git a/tests/set-image/no-image-name/Kptfile b/tests/set-image/nonunique-image-with-no-image-name-input/Kptfile similarity index 100% rename from tests/set-image/no-image-name/Kptfile rename to tests/set-image/nonunique-image-with-no-image-name-input/Kptfile diff --git a/tests/set-image/nonunique-image-with-no-image-name-input/resources.yaml b/tests/set-image/nonunique-image-with-no-image-name-input/resources.yaml new file mode 100644 index 000000000..6cb018f44 --- /dev/null +++ b/tests/set-image/nonunique-image-with-no-image-name-input/resources.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: Pod +metadata: + name: app1 +spec: + containers: + - name: server + image: nginx:1.20.2 + - name: store + image: postgres:14.1 \ No newline at end of file diff --git a/tests/set-image/unique-image-with-no-image-name-input/.expected/config.yaml b/tests/set-image/unique-image-with-no-image-name-input/.expected/config.yaml new file mode 100644 index 000000000..946fcf907 --- /dev/null +++ b/tests/set-image/unique-image-with-no-image-name-input/.expected/config.yaml @@ -0,0 +1,5 @@ +apiVersion: kpt.dev/v1 +kind: FunctionResultList +metadata: + name: fnresults +exitCode: 0 \ No newline at end of file diff --git a/tests/set-image/unique-image-with-no-image-name-input/.expected/diff.patch b/tests/set-image/unique-image-with-no-image-name-input/.expected/diff.patch new file mode 100644 index 000000000..909a42896 --- /dev/null +++ b/tests/set-image/unique-image-with-no-image-name-input/.expected/diff.patch @@ -0,0 +1,14 @@ +diff --git a/resources.yaml b/resources.yaml +index f674e0b..ae7d829 100644 +--- a/resources.yaml ++++ b/resources.yaml +@@ -5,6 +5,6 @@ metadata: + spec: + containers: + - name: server +- image: nginx:1.20.2 ++ image: bar:v1.0 + - name: store +- image: nginx:14.1 +\ No newline at end of file ++ image: bar:v1.0 \ No newline at end of file diff --git a/tests/set-image/unique-image-with-no-image-name-input/.expected/results.yaml b/tests/set-image/unique-image-with-no-image-name-input/.expected/results.yaml new file mode 100644 index 000000000..3c5ae71f7 --- /dev/null +++ b/tests/set-image/unique-image-with-no-image-name-input/.expected/results.yaml @@ -0,0 +1,11 @@ +apiVersion: kpt.dev/v1 +kind: FunctionResultList +metadata: + name: fnresults +exitCode: 0 +items: + - image: gcr.io/kpt-fn/set-image:unstable + exitCode: 0 + results: + - message: 'summary: updated a total of 2 image(s)' + severity: info \ No newline at end of file diff --git a/tests/set-image/unique-image-with-no-image-name-input/.krmignore b/tests/set-image/unique-image-with-no-image-name-input/.krmignore new file mode 100644 index 000000000..9d7a4007d --- /dev/null +++ b/tests/set-image/unique-image-with-no-image-name-input/.krmignore @@ -0,0 +1 @@ +.expected diff --git a/tests/set-image/unique-image-with-no-image-name-input/Kptfile b/tests/set-image/unique-image-with-no-image-name-input/Kptfile new file mode 100644 index 000000000..3bbda3777 --- /dev/null +++ b/tests/set-image/unique-image-with-no-image-name-input/Kptfile @@ -0,0 +1,10 @@ +apiVersion: kpt.dev/v1 +kind: Kptfile +metadata: + name: example +pipeline: + mutators: + - image: gcr.io/kpt-fn/set-image:unstable + configMap: + newName: bar + newTag: v1.0 diff --git a/tests/set-image/unique-image-with-no-image-name-input/resources.yaml b/tests/set-image/unique-image-with-no-image-name-input/resources.yaml new file mode 100644 index 000000000..f674e0b7a --- /dev/null +++ b/tests/set-image/unique-image-with-no-image-name-input/resources.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: Pod +metadata: + name: app1 +spec: + containers: + - name: server + image: nginx:1.20.2 + - name: store + image: nginx:14.1 \ No newline at end of file