feat(pci-kubernetes): kubernetes OIDC add optional fields #14523

Eric-ciccotti · 2024-12-10T15:37:11Z

Question	Answer
Branch?	`develop`
Bug fix?	no
New feature?	yes
Breaking change?	no
Tickets	Fix #TAPC-34
License	BSD 3-Clause

Try to keep pull requests small so they can be easily reviewed.
Commits are signed-off
Only FR translations have been updated
Branch is up-to-date with target branch
Lint has passed locally
Standalone app was ran and tested locally
Ticket reference is mentioned in linked commits (internal only)
Breaking change is mentioned in relevant commits

Description

This PR add optional fields for kubernetes OIDC provider

+  "pci_projects_project_kubernetes_details_service_update_oidc_provider_action_update": "Modifier",
+  "pci_projects_project_kubernetes_details_service_update_oidc_provider_action_cancel": "Annuler",
+  "pci_projects_project_kubernetes_details_service_update_oidc_provider_field_url": "Provider URL (Obligatoire)",
+  "pci_projects_project_kubernetes_details_service_update_oidc_provider_field_url_extra": "Saisissez ici l'URL de discovery de votre fournisseur, sans path. L'URL doit pointer vers le niveau précédent .well-known/openid-configuration . Vous devez utiliser une URL débutant par https://",


Suggested change

"pci_projects_project_kubernetes_details_service_update_oidc_provider_field_url_extra": "Saisissez ici l'URL de discovery de votre fournisseur, sans path. L'URL doit pointer vers le niveau précédent .well-known/openid-configuration . Vous devez utiliser une URL débutant par https://",

"pci_projects_project_kubernetes_details_service_update_oidc_provider_field_url_extra": "Saisissez ici l'URL de discovery de votre fournisseur, sans path. L'URL doit pointer vers le niveau précédent .well-known/openid-configuration . Vous devez utiliser une URL commençant par https://",

nicolaslacroux · 2024-12-11T10:48:02Z

packages/manager/apps/pci-kubernetes/public/translations/oidc-provider/Messages_fr_FR.json

+  "pci_projects_project_kubernetes_details_service_hide_optional": "Voir moins d’options",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_ca_content_error": "Le champ caContent doit être une chaîne valide en Base64.",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_required_claim_error": "Le champ requiredClaim doit être au format 'clé=valeur'.",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_groups_claim_caption": "Ajouter plusieurs valeurs séparées par des virgules, exemple : valeur1, valeur2.",


Suggested change

"pci_projects_project_kubernetes_details_service_oidc_provider_field_groups_claim_caption": "Ajouter plusieurs valeurs séparées par des virgules, exemple : valeur1, valeur2.",

"pci_projects_project_kubernetes_details_service_oidc_provider_field_groups_claim_caption": "Ajoutez plusieurs valeurs en les séparant par des virgules, par exemple : group1, group2",

nicolaslacroux · 2024-12-11T10:57:09Z

packages/manager/apps/pci-kubernetes/public/translations/oidc-provider/Messages_fr_FR.json

+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_ca_content_error": "Le champ caContent doit être une chaîne valide en Base64.",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_required_claim_error": "Le champ requiredClaim doit être au format 'clé=valeur'.",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_groups_claim_caption": "Ajouter plusieurs valeurs séparées par des virgules, exemple : valeur1, valeur2.",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_required_claim_caption": "Ajouter plusieurs valeurs séparées par des virgules, exemple : clé1=valeur1, clé2=valeur2.",


Suggested change

"pci_projects_project_kubernetes_details_service_oidc_provider_field_required_claim_caption": "Ajouter plusieurs valeurs séparées par des virgules, exemple : clé1=valeur1, clé2=valeur2.",

"pci_projects_project_kubernetes_details_service_oidc_provider_field_required_claim_caption": "Ajoutez plusieurs valeurs en les séparant par des virgules, par exemple : role=admin, environment=production",

nicolaslacroux · 2024-12-11T11:09:28Z

packages/manager/apps/pci-kubernetes/public/translations/oidc-provider/Messages_fr_FR.json

+  "pci_projects_project_kubernetes_details_service_show_optional": "Configuration optionnelle",
+  "pci_projects_project_kubernetes_details_service_hide_optional": "Voir moins d’options",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_ca_content_error": "Le champ caContent doit être une chaîne valide en Base64.",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_required_claim_error": "Le champ requiredClaim doit être au format 'clé=valeur'.",


Add a error case if customer doesn't use "," as separator.

Le champ requiredClaim doit contenir une liste de valeurs séparées par des virgules, par exemple :role=admin, environment=production

nicolaslacroux · 2024-12-11T11:27:38Z

packages/manager/apps/pci-kubernetes/public/translations/oidc-provider/Messages_fr_FR.json

+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_required_claim_error": "Le champ requiredClaim doit être au format 'clé=valeur'.",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_groups_claim_caption": "Ajouter plusieurs valeurs séparées par des virgules, exemple : valeur1, valeur2.",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_required_claim_caption": "Ajouter plusieurs valeurs séparées par des virgules, exemple : clé1=valeur1, clé2=valeur2.",
+  "pci_projects_project_kubernetes_details_service_oidc_provider_field_groups_claim_error": "Le champ groupsClaim doit contenir une liste de valeurs séparées par des virgules."


Suggested change

"pci_projects_project_kubernetes_details_service_oidc_provider_field_groups_claim_error": "Le champ groupsClaim doit contenir une liste de valeurs séparées par des virgules."

"pci_projects_project_kubernetes_details_service_oidc_provider_field_groups_claim_error": "Le champ groupsClaim doit contenir une liste de valeurs séparées par des virgules, par exemple : group1, group2"

fredericvilcot · 2024-12-11T11:15:42Z

.../manager/apps/pci-kubernetes/src/pages/oidc-provider/update/UpdateOIDCProvider.page.spec.tsx

-    });
-  });
-});
+// import { describe, it, vi } from 'vitest';


As far as I see, this file should be removed.

fredericvilcot · 2024-12-11T11:17:14Z

packages/manager/apps/pci-kubernetes/src/types/index.ts

+// Fonction pour vérifier si une chaîne est une chaîne Base64 valide
+function isBase64(str: string) {
+  try {
+    return btoa(atob(str)) === str;
+  } catch (err) {
+    return false;
+  }
+}
+


There are multiple issues here:

we shouldn't declare any logical function into a types folder

the comment is in French

there is no test associated

fredericvilcot · 2024-12-11T11:21:05Z

packages/manager/apps/pci-kubernetes/src/helpers/index.ts

+export const camelToSnake = (camelCase: string): string =>
+  camelCase.replace(/([A-Z])/g, '_$1').toLowerCase();
+
+export const filterSchemaKeys = (schema, excludeKeys) =>


Missing type parameters

fredericvilcot · 2024-12-11T11:21:34Z

packages/manager/apps/pci-kubernetes/src/helpers/index.ts

+  Object.keys(schema.shape).filter((key) => !excludeKeys.includes(key));
+
+export const parseCommaSeparated = (
+  value: string | string[] | undefined,


Suggested change

value: string | string[] | undefined,

value?: string | string[],

fredericvilcot · 2024-12-11T11:22:36Z

packages/manager/apps/pci-kubernetes/src/helpers/index.ts

+      .map((item) => item.trim())
+      .filter(Boolean);


This is redundant with the function line 136, please make a dedicated function

fredericvilcot · 2024-12-11T11:33:16Z

packages/manager/apps/pci-kubernetes/src/components/oidc/OidcProviderModal.component.tsx

-    'upsert-oidc-provider',
-    'add-oidc-provider',
-    'update-oidc-provider',
+    // 'upsert-oidc-provider',


To be removed

fredericvilcot · 2024-12-11T11:34:48Z

packages/manager/apps/pci-kubernetes/src/components/oidc/OidcProviderModal.component.tsx

-    upsertOidcProvider(data);
+  const fields = useFormFields();
+
+  const onSubmit = (data: TOidcFormValues) => {


Wrap into a useCallback as upsertOidcProvider depends on a hook call

fredericvilcot · 2024-12-11T11:38:18Z

packages/manager/apps/pci-kubernetes/src/components/oidc/OidcProviderModal.component.tsx

-                variant={ODS_BUTTON_VARIANT.ghost}
-                onClick={onClose}
-                data-testid={`${mode}-oidc-provider:${mode}OIDCProvider-button_cancel`}
+                <OsdsText


There are a lot of redundancies in these elements, please think to refactor code using high level functions and/or React.CloneELement to prevent from rewriting the same things with different props.

fredericvilcot · 2024-12-11T11:39:08Z

packages/manager/apps/pci-kubernetes/src/components/oidc/TextAreaFormField.component.tsx

+import React, { FormEvent } from 'react';
+import { OsdsTextarea } from '@ovhcloud/ods-components/react';
+
+export type TtextAreaFormFieldProps = React.ComponentProps<


Suggested change

export type TtextAreaFormFieldProps = React.ComponentProps<

export type TTextAreaFormFieldProps = React.ComponentProps<

fredericvilcot · 2024-12-11T11:39:58Z

packages/manager/apps/pci-kubernetes/src/hooks/useFormField.ts

+const useFormFields = () => {
+  const { t } = useTranslation('oidc-provider');
+
+  const fields = useMemo(() => {


Suggested change

const fields = useMemo(() => {

return useMemo(() => {

JacquesLarique · 2024-12-16T13:12:20Z

...es/manager/apps/pci-kubernetes/src/components/service/ClusterAccessAndSecurity.component.tsx

@@ -1,4 +1,3 @@
-import { ResponseAPIError } from '@ovh-ux/manager-pci-common';


It seems this is still used (line 85-86)

ref: TAPC-2088 Signed-off-by: Eric Ciccotti <[email protected]>

ref: TAPC-34 Signed-off-by: Eric Ciccotti <[email protected]>

Signed-off-by: CDS Translator Agent <[email protected]>

sonarcloud · 2024-12-17T08:30:02Z

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.1% Duplication on New Code

See analysis details on SonarQube Cloud

Eric-ciccotti requested review from a team as code owners December 10, 2024 15:37

Eric-ciccotti requested review from sidlynx, kqesar, anooparveti, darsene and mhelhali-soufien December 10, 2024 15:37

Eric-ciccotti changed the base branch from master to develop December 10, 2024 15:38

Eric-ciccotti added the draft work in progress label Dec 10, 2024

github-actions bot added translation required feature New feature labels Dec 10, 2024

nicolaslacroux reviewed Dec 11, 2024

View reviewed changes

fredericvilcot requested changes Dec 11, 2024

View reviewed changes

Eric-ciccotti force-pushed the feat/pci-kubernetes_oidc-add-optional-fields_tapc-34 branch 2 times, most recently from f7d4b3e to c5b2e5d Compare December 16, 2024 08:48

Eric-ciccotti requested a review from fredericvilcot December 16, 2024 08:48

Eric-ciccotti force-pushed the feat/pci-kubernetes_oidc-add-optional-fields_tapc-34 branch from c5b2e5d to 42106df Compare December 16, 2024 13:00

JacquesLarique reviewed Dec 16, 2024

View reviewed changes

Eric-ciccotti force-pushed the feat/pci-kubernetes_oidc-add-optional-fields_tapc-34 branch 2 times, most recently from 7bac475 to 7839844 Compare December 16, 2024 13:54

JacquesLarique previously approved these changes Dec 16, 2024

View reviewed changes

Eric Ciccotti added 4 commits December 16, 2024 16:01

feat(pci.kubernetes): refacto oidc provider

2f0fada

ref: TAPC-2088 Signed-off-by: Eric Ciccotti <[email protected]>

feat(pci.kubernetes): fix PR return

21873c4

ref: TAPC-2088 Signed-off-by: Eric Ciccotti <[email protected]>

feat(pci.kubernetes): working wip

07cc208

ref: TAPC-34 Signed-off-by: Eric Ciccotti <[email protected]>

feat(pci.kubernetes): fix PR return

181e6f8

ref: TAPC-34 Signed-off-by: Eric Ciccotti <[email protected]>

Eric-ciccotti force-pushed the feat/pci-kubernetes_oidc-add-optional-fields_tapc-34 branch from 7839844 to 181e6f8 Compare December 16, 2024 15:01

feat(pci.kubernetes): fix build fail

6914dd0

ref: TAPC-34 Signed-off-by: Eric Ciccotti <[email protected]>

Eric-ciccotti dismissed JacquesLarique’s stale review via 6914dd0 December 16, 2024 16:12

Eric-ciccotti added the quality check label Dec 17, 2024

Eric-ciccotti removed the draft work in progress label Dec 17, 2024

fix(i18n): add missing translations [CDS 156]

a0a58c2

Signed-off-by: CDS Translator Agent <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pci-kubernetes): kubernetes OIDC add optional fields #14523

feat(pci-kubernetes): kubernetes OIDC add optional fields #14523

Eric-ciccotti commented Dec 10, 2024 •

edited

Loading

nicolaslacroux Dec 11, 2024

nicolaslacroux Dec 11, 2024

nicolaslacroux Dec 11, 2024

nicolaslacroux Dec 11, 2024

nicolaslacroux Dec 11, 2024

fredericvilcot Dec 11, 2024

fredericvilcot Dec 11, 2024

fredericvilcot Dec 11, 2024

fredericvilcot Dec 11, 2024

fredericvilcot Dec 11, 2024

fredericvilcot Dec 11, 2024

fredericvilcot Dec 11, 2024

fredericvilcot Dec 11, 2024

fredericvilcot Dec 11, 2024

fredericvilcot Dec 11, 2024

JacquesLarique Dec 16, 2024

sonarcloud bot commented Dec 17, 2024

	"pci_projects_project_kubernetes_details_service_update_oidc_provider_field_url_extra": "Saisissez ici l'URL de discovery de votre fournisseur, sans path. L'URL doit pointer vers le niveau précédent .well-known/openid-configuration . Vous devez utiliser une URL débutant par https://",
	"pci_projects_project_kubernetes_details_service_update_oidc_provider_field_url_extra": "Saisissez ici l'URL de discovery de votre fournisseur, sans path. L'URL doit pointer vers le niveau précédent .well-known/openid-configuration . Vous devez utiliser une URL commençant par https://",

	value: string \| string[] \| undefined,
	value?: string \| string[],

	export type TtextAreaFormFieldProps = React.ComponentProps<
	export type TTextAreaFormFieldProps = React.ComponentProps<

		@@ -1,4 +1,3 @@
		import { ResponseAPIError } from '@ovh-ux/manager-pci-common';

feat(pci-kubernetes): kubernetes OIDC add optional fields #14523

Are you sure you want to change the base?

feat(pci-kubernetes): kubernetes OIDC add optional fields #14523

Conversation

Eric-ciccotti commented Dec 10, 2024 • edited Loading

Description

Related

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 17, 2024

Quality Gate passed

Eric-ciccotti commented Dec 10, 2024 •

edited

Loading