Skip to content
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.

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

kustomize: Add support for OCI based helm repos #5167

Merged
merged 13 commits into from
Oct 17, 2023
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ see the [kubernetes documentation].

## Usage


### 1) Make a [kustomization] file

In some directory containing your YAML [resource]
Expand Down
12 changes: 10 additions & 2 deletions api/internal/builtins/HelmChartInflationGenerator.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

120 changes: 120 additions & 0 deletions api/krusty/helmchartinflationgenerator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,97 @@ spec:
type: ClusterIP
`

const expectedHelmExternalDNS = `
jkroepke marked this conversation as resolved.
Show resolved Hide resolved
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: external-dns
helm.sh/chart: external-dns-6.19.2
name: test-external-dns
namespace: default
spec:
replicas: 1
selector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: external-dns
template:
metadata:
annotations: null
labels:
app.kubernetes.io/instance: test
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: external-dns
helm.sh/chart: external-dns-6.19.2
spec:
affinity:
nodeAffinity: null
podAffinity: null
podAntiAffinity:
preferredDuringSchedulingIgnoredDuringExecution:
- podAffinityTerm:
labelSelector:
matchLabels:
app.kubernetes.io/instance: test
app.kubernetes.io/name: external-dns
topologyKey: kubernetes.io/hostname
weight: 1
containers:
- args:
- --metrics-address=:7979
- --log-level=info
- --log-format=text
- --policy=upsert-only
- --provider=aws
- --registry=txt
- --interval=1m
- --source=service
- --source=ingress
- --aws-api-retries=3
- --aws-zone-type=
- --aws-batch-change-size=1000
env:
- name: AWS_DEFAULT_REGION
value: us-east-1
envFrom: null
image: docker.io/bitnami/external-dns:0.13.4-debian-11-r14
imagePullPolicy: IfNotPresent
livenessProbe:
failureThreshold: 2
httpGet:
path: /healthz
port: http
initialDelaySeconds: 10
periodSeconds: 10
successThreshold: 1
timeoutSeconds: 5
name: external-dns
ports:
- containerPort: 7979
name: http
readinessProbe:
failureThreshold: 6
httpGet:
path: /healthz
port: http
initialDelaySeconds: 5
periodSeconds: 10
successThreshold: 1
timeoutSeconds: 5
resources:
limits: {}
requests: {}
volumeMounts: null
securityContext:
fsGroup: 1001
runAsUser: 1001
serviceAccountName: default
volumes: null
`

func TestHelmChartInflationGeneratorOld(t *testing.T) {
th := kusttest_test.MakeEnhancedHarnessWithTmpRoot(t)
defer th.Reset()
Expand Down Expand Up @@ -84,6 +175,35 @@ helmCharts:
th.AssertActualEqualsExpected(m, expectedHelm)
}

func TestHelmChartInflationGeneratorWithOciRepository(t *testing.T) {
th := kusttest_test.MakeEnhancedHarnessWithTmpRoot(t)
defer th.Reset()
if err := th.ErrIfNoHelm(); err != nil {
t.Skip("skipping: " + err.Error())
}

th.WriteK(th.GetRoot(), `
helmCharts:
- name: external-dns
repo: oci://registry-1.docker.io/bitnamicharts/
version: 6.19.2
releaseName: test
valuesInline:
crd:
create: false
rbac:
create: false
serviceAccount:
create: false
service:
enabled: false

`)

m := th.Run(th.GetRoot(), th.MakeOptionsPluginsEnabled())
th.AssertActualEqualsExpected(m, expectedHelmExternalDNS)
}

// Last mile helm - show how kustomize puts helm charts into different
// namespaces with different customizations.
func TestHelmChartProdVsDev(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,16 @@ func (p *plugin) pullCommand() []string {
"pull",
"--untar",
"--untardir", p.absChartHome(),
"--repo", p.Repo,
p.Name}
}
if p.Repo != "" {
if strings.Contains(p.Repo, "oci://") {
jkroepke marked this conversation as resolved.
Show resolved Hide resolved
args = append(args, fmt.Sprintf("%s/%s", strings.TrimRight(p.Repo, "/"), p.Name))
jkroepke marked this conversation as resolved.
Show resolved Hide resolved
jkroepke marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a test where the OCI repo doesn't end with a "/"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I think @annasong20 wants to add two type tests one of an OCI repo doesn't end with a "/" and the OCI repo ends with a "/" word.
Could you add another type of test?

Copy link
Member

Choose a reason for hiding this comment

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

@jkroepke
Could you check it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

} else {
args = append(args, "--repo", p.Repo, p.Name)
}
} else {
args = append(args, p.Name)
}
jkroepke marked this conversation as resolved.
Show resolved Hide resolved
if p.Version != "" {
args = append(args, "--version", p.Version)
}
Expand Down