Skip to content

Commit a21df32

Browse files
grolugrolu
committed
PR Feedback #1 
1 parent fd3bd30 commit a21df32

File tree

5 files changed

+147
-40
lines changed

5 files changed

+147
-40
lines changed

‎frontend/__tests__/composables/__snapshots__/useShootDns.spec.js.snap‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ exports[`composables > useShootDns > should add extension custom domain dns prov
2323
"kind": "DNSConfig",
2424
"providers": [
2525
{
26-
"credentials": "shoot-dns-service-bar",
26+
"credentials": "shoot-dns-service-s-bar",
2727
"domains": {
2828
"include": [
2929
"example.org",
@@ -39,7 +39,7 @@ exports[`composables > useShootDns > should add extension custom domain dns prov
3939
],
4040
"resources": [
4141
{
42-
"name": "shoot-dns-service-bar",
42+
"name": "shoot-dns-service-s-bar",
4343
"resourceRef": {
4444
"apiVersion": "v1",
4545
"kind": "Secret",
@@ -62,7 +62,7 @@ exports[`composables > useShootDns > should add extension dns providers 1`] = `
6262
"kind": "DNSConfig",
6363
"providers": [
6464
{
65-
"credentials": "shoot-dns-service-aws-route53-secret",
65+
"credentials": "shoot-dns-service-s-aws-route53-secret",
6666
"type": "aws-route53",
6767
},
6868
],
@@ -73,7 +73,7 @@ exports[`composables > useShootDns > should add extension dns providers 1`] = `
7373
],
7474
"resources": [
7575
{
76-
"name": "shoot-dns-service-aws-route53-secret",
76+
"name": "shoot-dns-service-s-aws-route53-secret",
7777
"resourceRef": {
7878
"apiVersion": "v1",
7979
"kind": "Secret",
@@ -141,7 +141,7 @@ exports[`composables > useShootDns > should delete extension dns providers > two
141141
"kind": "DNSConfig",
142142
"providers": [
143143
{
144-
"credentials": "shoot-dns-service-aws-route53-secret",
144+
"credentials": "shoot-dns-service-s-aws-route53-secret",
145145
"type": "aws-route53",
146146
},
147147
{
@@ -156,7 +156,7 @@ exports[`composables > useShootDns > should delete extension dns providers > two
156156
],
157157
"resources": [
158158
{
159-
"name": "shoot-dns-service-aws-route53-secret",
159+
"name": "shoot-dns-service-s-aws-route53-secret",
160160
"resourceRef": {
161161
"apiVersion": "v1",
162162
"kind": "Secret",
@@ -176,7 +176,7 @@ exports[`composables > useShootDns > should fallback to provider credentialsRef
176176
"kind": "DNSConfig",
177177
"providers": [
178178
{
179-
"credentials": "shoot-dns-service-bar",
179+
"credentials": "shoot-dns-service-s-bar",
180180
"type": "foo",
181181
},
182182
],
@@ -187,7 +187,7 @@ exports[`composables > useShootDns > should fallback to provider credentialsRef
187187
],
188188
"resources": [
189189
{
190-
"name": "shoot-dns-service-bar",
190+
"name": "shoot-dns-service-s-bar",
191191
"resourceRef": {
192192
"apiVersion": "v1",
193193
"kind": "Secret",

‎frontend/__tests__/composables/useShootDns.spec.js‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import { useCredentialStore } from '@/store/credential'
1717

1818
import { useShootDns } from '@/composables/useShootDns'
1919

20+
import cloneDeep from 'lodash/cloneDeep'
21+
2022
describe('composables', () => {
2123
describe('useShootDns', () => {
2224
const manifest = reactive({})
@@ -284,5 +286,86 @@ describe('composables', () => {
284286

285287
expect(manifest.spec).toMatchSnapshot()
286288
})
289+
290+
it('should generate distinct dns service resources for same-name credentials with different kinds', () => {
291+
const credentialStore = useCredentialStore()
292+
const credentials = cloneDeep(global.fixtures.credentials)
293+
credentials.secrets.push({
294+
apiVersion: 'v1',
295+
kind: 'Secret',
296+
metadata: {
297+
namespace: 'garden-test',
298+
name: 'shared-name',
299+
uid: 'secret-shared-name-uid',
300+
labels: {
301+
'dashboard.gardener.cloud/dnsProviderType': 'aws-route53',
302+
},
303+
},
304+
})
305+
credentials.workloadIdentities.push({
306+
apiVersion: 'security.gardener.cloud/v1alpha1',
307+
kind: 'WorkloadIdentity',
308+
metadata: {
309+
namespace: 'garden-test',
310+
name: 'shared-name',
311+
uid: 'wlid-shared-name-uid',
312+
labels: {
313+
'provider.extensions.gardener.cloud/aws-route53': 'true',
314+
},
315+
},
316+
spec: {
317+
targetSystem: {
318+
type: 'foo-infra',
319+
},
320+
},
321+
})
322+
credentialStore._setCredentials(credentials)
323+
324+
shootDns.addDnsServiceExtensionProvider({
325+
type: 'aws-route53',
326+
credentialsRef: {
327+
apiVersion: 'v1',
328+
kind: 'Secret',
329+
name: 'shared-name',
330+
},
331+
})
332+
shootDns.addDnsServiceExtensionProvider({
333+
type: 'aws-route53',
334+
credentialsRef: {
335+
apiVersion: 'security.gardener.cloud/v1alpha1',
336+
kind: 'WorkloadIdentity',
337+
name: 'shared-name',
338+
},
339+
})
340+
341+
expect(manifest.spec.resources).toEqual([
342+
{
343+
name: 'shoot-dns-service-s-shared-name',
344+
resourceRef: {
345+
apiVersion: 'v1',
346+
kind: 'Secret',
347+
name: 'shared-name',
348+
},
349+
},
350+
{
351+
name: 'shoot-dns-service-wlid-shared-name',
352+
resourceRef: {
353+
apiVersion: 'security.gardener.cloud/v1alpha1',
354+
kind: 'WorkloadIdentity',
355+
name: 'shared-name',
356+
},
357+
},
358+
])
359+
expect(shootDns.dnsServiceExtensionProviders).toEqual([
360+
{
361+
type: 'aws-route53',
362+
credentials: 'shoot-dns-service-s-shared-name',
363+
},
364+
{
365+
type: 'aws-route53',
366+
credentials: 'shoot-dns-service-wlid-shared-name',
367+
},
368+
])
369+
})
287370
})
288371
})

‎frontend/src/components/ShootDetails/GShootInfrastructureCard.vue‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,6 @@ import { useCloudProviderBinding } from '@/composables/credential/useCloudProvid
283283
import { useOpenStackConstraints } from '@/composables/useCloudProfile/useOpenStackConstraints'
284284
import {
285285
dnsProviderCredentialsRef,
286-
resolvedDnsProviderCredentialKind,
287286
dnsExtensionProviderResourceName,
288287
} from '@/composables/credential/helper'
289288
@@ -438,22 +437,13 @@ export default {
438437
return 'generated'
439438
},
440439
shootDnsPrimaryProviderCredential () {
440+
const credentialsRef = dnsProviderCredentialsRef(this.shootDnsPrimaryProvider)
441441
return this.getCredentialByRef({
442-
name: this.shootDnsPrimaryProviderCredentialName,
443-
kind: this.shootDnsPrimaryProviderCredentialKind,
442+
name: credentialsRef?.name,
443+
kind: credentialsRef?.kind,
444444
namespace: this.shootNamespace,
445445
})
446446
},
447-
shootDnsPrimaryProviderCredentialKind () {
448-
return resolvedDnsProviderCredentialKind({
449-
provider: this.shootDnsPrimaryProvider,
450-
extensionProviders: this.shootDnsServiceExtensionProviders,
451-
getResourceRef: this.getResourceRef,
452-
})
453-
},
454-
shootDnsPrimaryProviderCredentialName () {
455-
return dnsProviderCredentialsRef(this.shootDnsPrimaryProvider)?.name
456-
},
457447
},
458448
methods: {
459449
getCredentialByRef ({ name, kind, namespace }) {

‎frontend/src/composables/credential/helper.js‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import { decodeBase64 } from '@/utils'
88

99
import get from 'lodash/get'
10-
import find from 'lodash/find'
1110

1211
// Credentials
1312
export function isSecret (credential) {
@@ -78,15 +77,27 @@ export function dnsExtensionProviderResourceName (provider) {
7877
return provider?.credentials ?? provider?.secretName
7978
}
8079

81-
export function resolvedDnsProviderCredentialKind ({ provider, extensionProviders, getResourceRef }) {
82-
const credentialsRef = dnsProviderCredentialsRef(provider)
83-
const matchingExtensionProvider = find(extensionProviders, extensionProvider => {
84-
return getResourceRef(dnsExtensionProviderResourceName(extensionProvider))?.name === credentialsRef?.name
85-
})
80+
export function dnsCredentialResourceNamePart (credentialRef) {
81+
const kind = credentialRef?.kind
82+
const name = credentialRef?.name
83+
if (!kind || !name) {
84+
return undefined
85+
}
86+
87+
let kindPrefix
88+
switch (kind) {
89+
case 'Secret':
90+
kindPrefix = 's'
91+
break
92+
case 'WorkloadIdentity':
93+
kindPrefix = 'wlid'
94+
break
95+
default:
96+
kindPrefix = kind.toLowerCase()
97+
break
98+
}
8699

87-
return matchingExtensionProvider
88-
? getResourceRef(dnsExtensionProviderResourceName(matchingExtensionProvider))?.kind
89-
: credentialsRef?.kind
100+
return `${kindPrefix}-${name}`
90101
}
91102

92103
// Bindings

‎frontend/src/composables/useShootDns.js‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { useCloudProviderEntityList } from '@/composables/credential/useCloudPro
1919
import {
2020
dnsProviderCredentialsRef,
2121
dnsExtensionProviderResourceName,
22+
dnsCredentialResourceNamePart,
2223
} from '@/composables/credential/helper'
2324

2425
import get from 'lodash/get'
@@ -211,9 +212,10 @@ export const useShootDns = (manifest, options) => {
211212
: false
212213
})
213214

214-
function getDnsServiceExtensionResourceName (credentialName) {
215-
return credentialName
216-
? `shoot-dns-service-${credentialName}`
215+
function getDnsServiceExtensionResourceName (credentialRef) {
216+
const key = dnsCredentialResourceNamePart(credentialRef)
217+
return key
218+
? `shoot-dns-service-${key}`
217219
: undefined
218220
}
219221

@@ -234,24 +236,45 @@ export const useShootDns = (manifest, options) => {
234236
// find unused credential
235237
const usedResourceNames = map(resources.value, 'name')
236238
const credential = find(credentials.value, credential => {
237-
const resourceName = getDnsServiceExtensionResourceName(credential?.metadata?.name)
239+
const resourceName = getDnsServiceExtensionResourceName({
240+
kind: credential?.kind,
241+
name: credential?.metadata?.name,
242+
})
238243
return !includes(usedResourceNames, resourceName)
239244
})
240245

241-
return credential ? credential?.metadata?.name : undefined
246+
return credential
242247
}
243248

244249
function addDnsServiceExtensionProvider (options = {}) {
245250
normalizeDnsServiceExtensionProviders()
246251
normalizeDnsProviderCredentialsRefs()
247252

248253
const type = options.type ?? head(gardenerExtensionStore.dnsProviderTypes)
249-
const credentialName = options.credentialsRef?.name ?? options.secretName ?? getDefaultCredentialName(type)
254+
const defaultCredential = getDefaultCredentialName(type)
255+
const credentialsRef = options.credentialsRef ?? (options.secretName
256+
? {
257+
apiVersion: 'v1',
258+
kind: 'Secret',
259+
name: options.secretName,
260+
}
261+
: defaultCredential
262+
? {
263+
apiVersion: defaultCredential.apiVersion,
264+
kind: defaultCredential.kind,
265+
name: defaultCredential.metadata?.name,
266+
}
267+
: undefined)
250268

251269
const credentials = useCloudProviderEntityList(toRef(type), { credentialStore, gardenerExtensionStore, cloudProfileStore })
252-
const credential = find(credentials.value, ['metadata.name', credentialName])
270+
const credential = find(credentials.value, credential => {
271+
return dnsCredentialResourceNamePart({
272+
kind: credential?.kind,
273+
name: credential?.metadata?.name,
274+
}) === dnsCredentialResourceNamePart(credentialsRef)
275+
})
253276

254-
const resourceName = getDnsServiceExtensionResourceName(credentialName)
277+
const resourceName = getDnsServiceExtensionResourceName(credentialsRef)
255278
const provider = {
256279
type,
257280
credentials: resourceName,
@@ -266,9 +289,9 @@ export const useShootDns = (manifest, options) => {
266289
? {
267290
apiVersion: credential.apiVersion,
268291
kind: credential.kind,
269-
name: credentialName,
292+
name: credential.metadata?.name,
270293
}
271-
: options.credentialsRef
294+
: credentialsRef
272295

273296
if (resourceName && resourceRef) {
274297
setResource({

0 commit comments

Comments
 (0)