Skip to content

Templatize image pull secrets and provide value from values.yaml instead of permanently hardcoding them #177

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Templatize image pull secrets and provide value from values.yaml instead of permanently hardcoding them #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17b8ab8
Add field for imagePullSecrets if image pull secrets are enabled
msafwankarim Oct 31, 2025
c2f8669
Fixed test cases
msafwankarim Oct 31, 2025
3c73c71
Merge branch 'main' into issue-176-include-image-pull-secrets
msafwankarim Nov 26, 2025
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
10 changes: 7 additions & 3 deletions pkg/processor/pod/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/arttor/helmify/pkg/helmify"
securityContext "github.com/arttor/helmify/pkg/processor/security-context"
"github.com/iancoleman/strcase"
"github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -54,10 +55,13 @@ func ProcessSpec(objName string, appMeta helmify.AppMetadata, spec corev1.PodSpe
}

if appMeta.Config().ImagePullSecrets {
if _, defined := specMap["imagePullSecrets"]; !defined {
specMap["imagePullSecrets"] = "{{ .Values.imagePullSecrets | default list | toJson }}"
values["imagePullSecrets"] = []string{}
sourceImagePullSecrets, ok := specMap["imagePullSecrets"].([]interface{})
if !ok {
logrus.Debug("imagePullSecrets not found in pod spec, setting empty map")
sourceImagePullSecrets = []interface{}{}
}
specMap["imagePullSecrets"] = "{{ .Values.imagePullSecrets | default list | toJson }}"
values["imagePullSecrets"] = sourceImagePullSecrets
Copy link
Owner

@arttor arttor Nov 1, 2025

Choose a reason for hiding this comment

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

what will happen if there are 2 pods with different imagePullSecrets in the chart?

Copy link
Author

@msafwankarim msafwankarim Nov 1, 2025

Choose a reason for hiding this comment

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

Currently it will pickup the last one. Should we merge them into one list? Other option is to keep the list empty if we find more than one secrets. This will result in same behavior as not providing any secrets by default (i.e. current behavior)

Copy link
Owner

@arttor arttor Nov 1, 2025

Choose a reason for hiding this comment

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

i dont know what would be the best solution. maybe create a separate values for every pod. for example, image name is extracted to values per pod/container.

I just wanted to highligh that the current approch will lead to non-determenistic behaviour and data loss for cases with multiple deploymetns using different imagePullSecrets.

Copy link
Author

@msafwankarim msafwankarim Nov 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I've a scenario where I've manifests of multiple workloads all being fetched from private repo. Personally I think it will be annoying to provide same secret for N number of times. But I'll leave final decision to you. As long as we've a way to override this value from values.yml, I'm ok with it.

Other option I thought of was instead of keeping image-pull-secrets parameter boolean we make it an enum.

  1. Preserve (keep the current behavior that is already implemented)
  2. Merged (Add one pull secrets field in values.yml with all secrets to be used in all workloads)
  3. Indvidual (Have separate field for each workload)

}

err = securityContext.ProcessContainerSecurityContext(objName, specMap, &values, nindent)
Expand Down
77 changes: 77 additions & 0 deletions pkg/processor/pod/pod_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package pod
import (
"testing"

"github.com/arttor/helmify/pkg/config"
"github.com/arttor/helmify/pkg/helmify"
"github.com/arttor/helmify/pkg/metadata"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -137,6 +138,34 @@ spec:
runAsNonRoot: true
runAsUser: 65532

`
strDeploymentWithImagePullSecrets = `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
labels:
app: nginx
spec:
replicas: 3
selector:
matchLabels:
app: nginx
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: nginx:1.14.2
args:
- --test
- --arg
ports:
- containerPort: 80
imagePullSecrets:
- name: myregistrykey
`
)

Expand Down Expand Up @@ -376,5 +405,53 @@ func Test_pod_Process(t *testing.T) {
},
}, tmpl)
})
t.Run("deployment without imagePullSecrets", func(t *testing.T) {
var deploy appsv1.Deployment
obj := internal.GenerateObj(strDeployment)
err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &deploy)
assert.NoError(t, err)
specMap, _, err := ProcessSpec("nginx", &metadata.Service{}, deploy.Spec.Template.Spec)
assert.NoError(t, err)

// when ImagePullSecrets is disabled in config, spec should not contain imagePullSecrets key
_, ok := specMap["imagePullSecrets"]
assert.False(t, ok)
})

t.Run("deployment with imagePullSecrets enabled but not provided in source", func(t *testing.T) {
var deploy appsv1.Deployment
obj := internal.GenerateObj(strDeployment)
err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &deploy)
assert.NoError(t, err)
// enable ImagePullSecrets in config via metadata.New but source doesn't include imagePullSecrets
svc := metadata.New(config.Config{ImagePullSecrets: true})
specMap, tmpl, err := ProcessSpec("nginx", svc, deploy.Spec.Template.Spec)
assert.NoError(t, err)

// spec should contain templated imagePullSecrets
assert.Equal(t, "{{ .Values.imagePullSecrets | default list | toJson }}", specMap["imagePullSecrets"])

assert.Equal(t, []interface{}{}, tmpl["imagePullSecrets"])
})

t.Run("deployment with imagePullSecrets", func(t *testing.T) {

var deploy appsv1.Deployment
obj := internal.GenerateObj(strDeploymentWithImagePullSecrets)
err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &deploy)
assert.NoError(t, err)
// enable ImagePullSecrets in config via metadata.New
svc := metadata.New(config.Config{ImagePullSecrets: true})
specMap, tmpl, err := ProcessSpec("nginx", svc, deploy.Spec.Template.Spec)
assert.NoError(t, err)

// spec should contain templated imagePullSecrets
assert.Equal(t, "{{ .Values.imagePullSecrets | default list | toJson }}", specMap["imagePullSecrets"])

// values should contain the original imagePullSecrets slice
assert.Equal(t, []interface{}{
map[string]interface{}{"name": "myregistrykey"},
}, tmpl["imagePullSecrets"])
})

}