Display OIDC observability URLs (#1899)

* drop removed spec.dns.ingressDomain Removed with gardener/gardener#7529 * use user client to fetch secret * Add oidcObservabilityUrlsEnabled frontend config * display OIDC observability urls if enabled * drop seedShootIngressDomain from shoot info * drop obsolete prop * do not show alertmanager when ignored * PR feedback * PR feedback II * PR feedback III
gardener · Jun 10, 2024 · edd68fd · edd68fd
1 parent 6af6ee8
commit edd68fd
Show file tree

Hide file tree

Showing 22 changed files with 214 additions and 105 deletions.
diff --git a/backend/__fixtures__/auth.js b/backend/__fixtures__/auth.js
@@ -126,10 +126,11 @@ const mocks = {
       const { id } = auth.getTokenPayload(headers)
       const { resourceAttributes, nonResourceAttributes } = json.spec
       if (resourceAttributes) {
-        const { resource } = resourceAttributes
+        const { resource, namespace } = resourceAttributes
         switch (resource) {
           case 'secrets':
-            allowed = id === '[email protected]'
+            allowed = id === '[email protected]' ||
+              (id === '[email protected]' && namespace === 'garden-foo')
             break
           case 'projects':
             allowed = id === '[email protected]'

diff --git a/backend/__fixtures__/seeds.js b/backend/__fixtures__/seeds.js
@@ -31,8 +31,8 @@ function getSeed ({
         type: kind,
         region
       },
-      dns: {
-        ingressDomain: `ingress.${region}.${kind}.example.org`
+      ingress: {
+        domain: `ingress.${region}.${kind}.example.org`
       },
       taints: [],
       settings: {

diff --git a/backend/lib/services/authorization.js b/backend/lib/services/authorization.js
@@ -135,6 +135,18 @@ exports.canListControllerRegistrations = function (user) {
   })
 }
 
+exports.canGetSecret = function (user, namespace, name) {
+  return hasAuthorization(user, {
+    resourceAttributes: {
+      verb: 'get',
+      group: '',
+      resource: 'secrets',
+      namespace,
+      name
+    }
+  })
+}
+
 /*
 SelfSubjectRulesReview should only be used to hide/show actions or views on the UI and not for authorization checks.
 */

diff --git a/backend/lib/services/seeds.js b/backend/lib/services/seeds.js
@@ -24,11 +24,13 @@ function fromResource (seed) {
   const visible = _.get(seed, 'spec.settings.scheduling.visible')
   const provider = _.get(seed, 'spec.provider')
   const volume = _.get(seed, 'spec.volume')
+  const ingressDomain = _.get(seed, 'spec.ingress.domain')
   const data = {
     volume,
     ...provider,
     visible,
-    unprotected
+    unprotected,
+    ingressDomain
   }
 
   return { metadata, data }

diff --git a/backend/lib/services/shoots.js b/backend/lib/services/shoots.js
@@ -23,7 +23,6 @@ const {
   decodeBase64,
   encodeBase64,
   getSeedNameFromShoot,
-  getSeedIngressDomain,
   projectFilter
 } = utils
 const { getSeed } = cache
@@ -334,13 +333,6 @@ exports.info = async function ({ user, namespace, name }) {
 
   if (shoot.spec.seedName) {
     const seed = getSeed(getSeedNameFromShoot(shoot))
-    const prefix = _.replace(shoot.status.technicalID, /^shoot-{1,2}/, '')
-    if (prefix) {
-      const ingressDomain = getSeedIngressDomain(seed)
-      if (ingressDomain) {
-        data.seedShootIngressDomain = `${prefix}.${ingressDomain}`
-      }
-    }
     if (seed && namespace !== 'garden') {
       try {
         data.canLinkToSeed = !!(await client['core.gardener.cloud'].shoots.get('garden', seed.metadata.name))
@@ -372,14 +364,10 @@ exports.info = async function ({ user, namespace, name }) {
   }
   data.dashboardUrlPath = getDashboardUrlPath(shoot.spec.kubernetes.version)
 
-  /*
-    We explicitly use the (privileged) dashboardClient here for fetching the monitoring credentials instead of using the user's token
-    as we agreed that also project viewers should be able to see the monitoring credentials.
-    Usually project viewers do not have the permission to read the <shootName>.monitoring credential.
-    Our assumption: if the user can read the shoot resource, the user can be considered as project viewer.
-    This is only a temporary workaround until a Plutono SSO solution is implemented https://github.com/gardener/monitoring/issues/11.
-  */
-  await assignMonitoringSecret(dashboardClient, data, namespace, name)
+  const oidcObservabilityUrlsEnabled = _.get(config, 'frontend.features.oidcObservabilityUrlsEnabled', false)
+  if (!oidcObservabilityUrlsEnabled && await authorization.canGetSecret(user, namespace, `${name}.monitoring`)) {
+    await assignMonitoringSecret(client, data, namespace, name)
+  }
 
   return data
 }

diff --git a/backend/lib/utils/index.js b/backend/lib/utils/index.js
@@ -181,7 +181,7 @@ function shootHasIssue (shoot) {
 }
 
 function getSeedIngressDomain (seed) {
-  return _.get(seed, 'spec.dns.ingressDomain') || _.get(seed, 'spec.ingress.domain')
+  return _.get(seed, 'spec.ingress.domain')
 }
 
 function isSeedUnreachable (seed) {

diff --git a/backend/test/acceptance/__snapshots__/api.seeds.spec.js.snap b/backend/test/acceptance/__snapshots__/api.seeds.spec.js.snap
@@ -30,6 +30,7 @@ exports[`api seeds should return all seeds 2`] = `
 Array [
   Object {
     "data": Object {
+      "ingressDomain": "ingress.foo-east.infra1.example.org",
       "region": "foo-east",
       "type": "infra1",
       "unprotected": false,
@@ -42,6 +43,7 @@ Array [
   },
   Object {
     "data": Object {
+      "ingressDomain": "ingress.foo-east.infra1.example.org",
       "region": "foo-east",
       "type": "infra1",
       "unprotected": true,
@@ -54,6 +56,7 @@ Array [
   },
   Object {
     "data": Object {
+      "ingressDomain": "ingress.foo-west.infra1.example.org",
       "region": "foo-west",
       "type": "infra1",
       "unprotected": true,
@@ -66,6 +69,7 @@ Array [
   },
   Object {
     "data": Object {
+      "ingressDomain": "ingress.foo-europe.infra3.example.org",
       "region": "foo-europe",
       "type": "infra3",
       "unprotected": true,
@@ -78,6 +82,7 @@ Array [
   },
   Object {
     "data": Object {
+      "ingressDomain": "ingress.foo-south.infra1.example.org",
       "region": "foo-south",
       "type": "infra1",
       "unprotected": true,
@@ -90,6 +95,7 @@ Array [
   },
   Object {
     "data": Object {
+      "ingressDomain": "ingress.foo-europe.infra3.example.org",
       "region": "foo-europe",
       "type": "infra3",
       "unprotected": true,
@@ -102,6 +108,7 @@ Array [
   },
   Object {
     "data": Object {
+      "ingressDomain": "ingress.foo-europe.infra3.example.org",
       "region": "foo-europe",
       "type": "infra3",
       "unprotected": true,
@@ -114,6 +121,7 @@ Array [
   },
   Object {
     "data": Object {
+      "ingressDomain": "ingress.foo-europe.infra3.example.org",
       "region": "foo-europe",
       "type": "infra3",
       "unprotected": false,
@@ -126,6 +134,7 @@ Array [
   },
   Object {
     "data": Object {
+      "ingressDomain": "ingress.foo-europe.infra3.example.org",
       "region": "foo-europe",
       "type": "infra3",
       "unprotected": true,

diff --git a/backend/test/acceptance/__snapshots__/api.shoots.spec.js.snap b/backend/test/acceptance/__snapshots__/api.shoots.spec.js.snap
@@ -1702,13 +1702,36 @@ Array [
       "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImZvb0BleGFtcGxlLm9yZyIsImlhdCI6MTU3NzgzNjgwMCwiYXVkIjpbImdhcmRlbmVyIl0sImV4cCI6MzE1NTcxNjgwMCwianRpIjoianRpIn0.k3kGjF6AgugJLdwERXEWZPaibFAPFPOnmpT3YM9H0xU",
     },
   ],
+  Array [
+    Object {
+      ":authority": "kubernetes:6443",
+      ":method": "post",
+      ":path": "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews",
+      ":scheme": "https",
+      "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImZvb0BleGFtcGxlLm9yZyIsImlhdCI6MTU3NzgzNjgwMCwiYXVkIjpbImdhcmRlbmVyIl0sImV4cCI6MzE1NTcxNjgwMCwianRpIjoianRpIn0.k3kGjF6AgugJLdwERXEWZPaibFAPFPOnmpT3YM9H0xU",
+    },
+    Object {
+      "apiVersion": "authorization.k8s.io/v1",
+      "kind": "SelfSubjectAccessReview",
+      "spec": Object {
+        "nonResourceAttributes": undefined,
+        "resourceAttributes": Object {
+          "group": "",
+          "name": "barShoot.monitoring",
+          "namespace": "garden-foo",
+          "resource": "secrets",
+          "verb": "get",
+        },
+      },
+    },
+  ],
   Array [
     Object {
       ":authority": "kubernetes:6443",
       ":method": "get",
       ":path": "/api/v1/namespaces/garden-foo/secrets/barShoot.monitoring",
       ":scheme": "https",
-      "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJrdWJlcm5ldGVzL3NlcnZpY2VhY2NvdW50Iiwic3ViIjoic3lzdGVtOnNlcnZpY2VhY2NvdW50OmdhcmRlbjpkZWZhdWx0In0.-4rSuvvj5BStN6DwnmLAaRVbgpl5iCn2hG0pcqx0NPw",
+      "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImZvb0BleGFtcGxlLm9yZyIsImlhdCI6MTU3NzgzNjgwMCwiYXVkIjpbImdhcmRlbmVyIl0sImV4cCI6MzE1NTcxNjgwMCwianRpIjoianRpIn0.k3kGjF6AgugJLdwERXEWZPaibFAPFPOnmpT3YM9H0xU",
     },
   ],
 ]
@@ -1743,13 +1766,36 @@ Array [
       "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImZvb0BleGFtcGxlLm9yZyIsImlhdCI6MTU3NzgzNjgwMCwiYXVkIjpbImdhcmRlbmVyIl0sImV4cCI6MzE1NTcxNjgwMCwianRpIjoianRpIn0.k3kGjF6AgugJLdwERXEWZPaibFAPFPOnmpT3YM9H0xU",
     },
   ],
+  Array [
+    Object {
+      ":authority": "kubernetes:6443",
+      ":method": "post",
+      ":path": "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews",
+      ":scheme": "https",
+      "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImZvb0BleGFtcGxlLm9yZyIsImlhdCI6MTU3NzgzNjgwMCwiYXVkIjpbImdhcmRlbmVyIl0sImV4cCI6MzE1NTcxNjgwMCwianRpIjoianRpIn0.k3kGjF6AgugJLdwERXEWZPaibFAPFPOnmpT3YM9H0xU",
+    },
+    Object {
+      "apiVersion": "authorization.k8s.io/v1",
+      "kind": "SelfSubjectAccessReview",
+      "spec": Object {
+        "nonResourceAttributes": undefined,
+        "resourceAttributes": Object {
+          "group": "",
+          "name": "dummyShoot.monitoring",
+          "namespace": "garden-foo",
+          "resource": "secrets",
+          "verb": "get",
+        },
+      },
+    },
+  ],
   Array [
     Object {
       ":authority": "kubernetes:6443",
       ":method": "get",
       ":path": "/api/v1/namespaces/garden-foo/secrets/dummyShoot.monitoring",
       ":scheme": "https",
-      "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJrdWJlcm5ldGVzL3NlcnZpY2VhY2NvdW50Iiwic3ViIjoic3lzdGVtOnNlcnZpY2VhY2NvdW50OmdhcmRlbjpkZWZhdWx0In0.-4rSuvvj5BStN6DwnmLAaRVbgpl5iCn2hG0pcqx0NPw",
+      "authorization": "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImZvb0BleGFtcGxlLm9yZyIsImlhdCI6MTU3NzgzNjgwMCwiYXVkIjpbImdhcmRlbmVyIl0sImV4cCI6MzE1NTcxNjgwMCwianRpIjoianRpIn0.k3kGjF6AgugJLdwERXEWZPaibFAPFPOnmpT3YM9H0xU",
     },
   ],
 ]
@@ -1762,7 +1808,6 @@ Object {
   "kubeconfig": Any<String>,
   "monitoringPassword": "pass-garden-foo-dummyShoot.monitoring",
   "monitoringUsername": "user-garden-foo-dummyShoot.monitoring",
-  "seedShootIngressDomain": "foo--dummyShoot.ingress.foo-south.infra1.example.org",
 }
 `;
 
@@ -1809,7 +1854,6 @@ Object {
   "kubeconfigGardenlogin": Any<String>,
   "monitoringPassword": "pass-garden-foo-barShoot.monitoring",
   "monitoringUsername": "user-garden-foo-barShoot.monitoring",
-  "seedShootIngressDomain": "foo--barShoot.ingress.foo-east.infra1.example.org",
 }
 `;
 

diff --git a/backend/test/acceptance/api.shoots.spec.js b/backend/test/acceptance/api.shoots.spec.js
@@ -210,6 +210,7 @@ describe('api', function () {
       mockRequest.mockImplementationOnce(fixtures.configmaps.mocks.get())
       mockRequest.mockImplementationOnce(fixtures.configmaps.mocks.get())
       mockRequest.mockImplementationOnce(fixtures.shoots.mocks.get())
+      mockRequest.mockImplementationOnce(fixtures.auth.mocks.reviewSelfSubjectAccess())
       mockRequest.mockImplementationOnce(fixtures.secrets.mocks.get())
 
       const res = await agent
@@ -218,7 +219,7 @@ describe('api', function () {
         .expect('content-type', /json/)
         .expect(200)
 
-      expect(mockRequest).toBeCalledTimes(6)
+      expect(mockRequest).toBeCalledTimes(7)
       expect(mockRequest.mock.calls).toMatchSnapshot()
 
       expect(kubeconfig.cleanKubeconfig).toBeCalledTimes(1)
@@ -239,6 +240,7 @@ describe('api', function () {
       mockRequest.mockImplementationOnce(fixtures.shoots.mocks.get())
       mockRequest.mockImplementationOnce(fixtures.secrets.mocks.get())
       mockRequest.mockImplementationOnce(fixtures.shoots.mocks.get())
+      mockRequest.mockImplementationOnce(fixtures.auth.mocks.reviewSelfSubjectAccess())
       mockRequest.mockImplementationOnce(fixtures.secrets.mocks.get())
 
       const res = await agent
@@ -247,7 +249,7 @@ describe('api', function () {
         .expect('content-type', /json/)
         .expect(200)
 
-      expect(mockRequest).toBeCalledTimes(4)
+      expect(mockRequest).toBeCalledTimes(5)
       expect(mockRequest.mock.calls).toMatchSnapshot()
 
       expect(kubeconfig.cleanKubeconfig).toBeCalledTimes(1)

diff --git a/...__tests__/gardener-dashboard/application/dashboard/__snapshots__/clusterrole.spec.js.snap b/...__tests__/gardener-dashboard/application/dashboard/__snapshots__/clusterrole.spec.js.snap
@@ -111,17 +111,6 @@ Object {
         "watch",
       ],
     },
-    Object {
-      "apiGroups": Array [
-        "",
-      ],
-      "resources": Array [
-        "secrets",
-      ],
-      "verbs": Array [
-        "get",
-      ],
-    },
   ],
 }
 `;
diff --git a/charts/__tests__/gardener-dashboard/runtime/dashboard/__snapshots__/configmap.spec.js.snap b/charts/__tests__/gardener-dashboard/runtime/dashboard/__snapshots__/configmap.spec.js.snap
@@ -422,6 +422,7 @@ Object {
       },
     ],
     "features": Object {
+      "oidcObservabilityUrlsEnabled": false,
       "projectTerminalShortcutsEnabled": false,
       "terminalEnabled": false,
     },

diff --git a/charts/gardener-dashboard/charts/application/templates/dashboard/clusterrole.yaml b/charts/gardener-dashboard/charts/application/templates/dashboard/clusterrole.yaml
@@ -65,11 +65,4 @@ rules:
   verbs:
   - list
   - watch
-# keep until gardener/monitoring#11 is resolved
-- apiGroups:
-  - ""
-  resources:
-  - secrets
-  verbs:
-  - get
 {{- end }}
diff --git a/charts/gardener-dashboard/charts/runtime/templates/dashboard/configmap.yaml b/charts/gardener-dashboard/charts/runtime/templates/dashboard/configmap.yaml
@@ -207,6 +207,7 @@ data:
       features:
         terminalEnabled: {{ .Values.global.dashboard.frontendConfig.features.terminalEnabled | default false }}
         projectTerminalShortcutsEnabled: {{ .Values.global.dashboard.frontendConfig.features.projectTerminalShortcutsEnabled | default false }}
+        oidcObservabilityUrlsEnabled: {{ .Values.global.dashboard.frontendConfig.features.oidcObservabilityUrlsEnabled | default false }}
       experimental:
         throttleDelayPerCluster: {{ .Values.global.dashboard.frontendConfig.experimental.throttleDelayPerCluster | default 10 }}
       {{- if .Values.global.dashboard.frontendConfig.terminal }}

diff --git a/charts/gardener-dashboard/values.yaml b/charts/gardener-dashboard/values.yaml
@@ -233,6 +233,10 @@ global:
       features:
         terminalEnabled: false
         projectTerminalShortcutsEnabled: false
+        # The oidcObservabilityUrlsEnabled feature flag controls whether OIDC observability URLs are displayed in the dashboard.
+        # This requires the gardener/oidc-apps-controller extension to be deployed and assumes the hostPrefix (https://github.com/gardener/oidc-apps-controller/blob/653add2e304c382c1b8ba4e4b775477e288826a3/example/config.yaml#L64) is empty.
+        # When enabled, it shows the OIDC URLs; when disabled, it displays the current observability URLs for basic auth login along with the credentials.
+        oidcObservabilityUrlsEnabled: false
 
       shootAdminKubeconfig:
         enabled: false