Skip to content

Commit

Permalink
fix(kubernetes): non-string annotation values cause the annotation ma…
Browse files Browse the repository at this point in the history
…p in a manifest's Go representation to be empty (#175)

* update go version in workflow

* force annotation values to be string when converting to manifest to kubernetes unstructured.Unstructured

---------

Co-authored-by: abe garcia <[email protected]>
  • Loading branch information
alice485 and abe garcia authored Jan 10, 2024
1 parent f12dcc4 commit 96ece6f
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 3 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
name: Build

on:
workflow_dispatch:
push:
branches: [master]
pull_request:
Expand All @@ -25,7 +26,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: ^1.13
go-version: "1.18"

- name: Get dependencies
run: |
Expand Down
1 change: 1 addition & 0 deletions internal/api/core/kubernetes/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func (cc *Controller) Deploy(c *gin.Context, dm DeployManifestRequest) {
clouddriver.Error(c, http.StatusBadRequest, err)
return
}

// Merge all list element items into the manifest list.
manifests, err = mergeManifests(manifests)
if err != nil {
Expand Down
25 changes: 25 additions & 0 deletions internal/api/core/kubernetes/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,31 @@ var _ = Describe("Deploy", func() {
})
})

When("the manifest contains a non-string annotation", func() {
BeforeEach(func() {
deployManifestRequest.Manifests = []map[string]interface{}{
{
"kind": "ConfigMap",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
kubernetes.AnnotationSpinnakerMaxVersionHistory: 10,
kubernetes.AnnotationSpinnakerReplaced: true,
},
"name": "test",
},
},
}
})

It("suceeds and annotations are set properly", func() {
Expect(c.Writer.Status()).To(Equal(http.StatusOK))
u := fakeKubeClient.ReplaceArgsForCall(0)
Expect(u.GetAnnotations()).To(HaveKeyWithValue(kubernetes.AnnotationSpinnakerMaxVersionHistory, "10"))
Expect(u.GetAnnotations()).To(HaveKeyWithValue(kubernetes.AnnotationSpinnakerReplaced, "true"))
})
})

Context("the kind is a list", func() {
var fakeUnstructured unstructured.Unstructured
var manifests []map[string]interface{}
Expand Down
28 changes: 26 additions & 2 deletions internal/kubernetes/unstructured.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package kubernetes

import (
"encoding/json"
"fmt"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -43,9 +44,32 @@ func ToUnstructured(manifest map[string]interface{}) (unstructured.Unstructured,
return unstructured.Unstructured{}, err
}

return unstructured.Unstructured{
u := unstructured.Unstructured{
Object: m,
}, nil
}

// Attempt to get annotations as map[string]string
// If no errors then nothing else needs to be done
if _, _, err := unstructured.NestedStringMap(u.Object, "metadata", "annotations"); err == nil {
return u, nil
}

// Attempt to get annotations as map[string]interface{}
annotationsMap, exists, err := unstructured.NestedMap(u.Object, "metadata", "annotations")
if err != nil || !exists {
return u, err
}

// If annotations exist in manifest and are map[string]interface, convert to map[string]string
annotations := make(map[string]string, len(annotationsMap))

for k, v := range annotationsMap {
annotations[k] = fmt.Sprintf("%v", v)
}

u.SetAnnotations(annotations)

return u, nil
}

func SetDefaultNamespaceIfScopedAndNoneSet(u *unstructured.Unstructured, helper *resource.Helper) {
Expand Down

0 comments on commit 96ece6f

Please sign in to comment.