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

Import contacts via SFTP (version bash) #794

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

Conversation

alanzirek
Copy link
Collaborator

@alanzirek alanzirek commented Mar 6, 2025

Cette PR ajoute une commande Django permettant de récupérer des fichiers chiffrés (fichier de contacts et une clé symétrique) sur un serveur SFTP puis d'importer les contacts en base.
La récupération des fichiers chiffrés et leurs déchiffrements se fait via un script bash (équivalent de la PR #765).

Ce qui a été fait :

Ajout sshpass

Ajout du buildpack apt-buildpack pour pouvoir ajouter la dépendance sshpass (requise pour l’exécution du script bash).
Document Scalingo : https://doc.scalingo.com/platform/deployment/buildpacks/apt
ATFU, CleverCloud permet l'authentification par clés SSH auprès du SFTP. À voir si l'on remplace la connexion au serveur SFTP via clés SSH à la place du username/mdp.

Clé publique et privée

En local, génération de la clé publique et privée, encodage en base64 de la clé privée et stockage dans la variable d'environnement SFTP_PRIVATE_KEY.
Documentation Scalingo : https://doc.scalingo.com/platform/app/secret-file-in-app#access-secret-file-from-the-application

Fichier known_hosts

En local, génération et encodage du fichier known_hosts avec les commandes suivantes :

Script bash

Le script consiste à :

  • se connecter sur le serveur SFTP pour récupérer en local les fichiers (données et clé symétrique) les plus récents,
  • déchiffrer la clé symétrique avec la clé privée,
  • déchiffrer le fichier de données (fichier agricoll.csv) avec la clé symétrique déchiffrée.

Commande Django

La commande Django consiste à :

  • vérifier la présence des différentes variables d'environnements nécessaire à l’exécution du script bash,
  • lancer l’exécution du script bash,
  • lancer la commande d'import des contacts (cf. fichier agricoll.csv généré via script bash).

Test

Un test a été ajouté pour tester le bon déroulement de toutes la procédure :

  • génération des clés privée et publique,
  • encodage en base64 et stockage de la clé privée dans la variable d'environnement SFTP_PRIVATE_KEY,
  • génération de deux fichiers de données,
  • génération de deux clés symétriques,
  • chiffrement des clés symétriques (avec la clé publique),
  • chiffrement des fichiers de données (avec clés symétriques),
  • upload des fichiers de données et clés symétriques sur le serveur SFTP (pour pouvoir tester la récupération des derniers fichiers uploadés),
  • récupération des derniers fichiers chiffrés uploadés,
  • déchiffrement des fichiers,
  • import des contacts.

Pour le test, la dépendance testcontainer[sftp] a été ajoutée pour lancer un serveur SFTP (container docker) pour être iso prod. Fonctionne sur la CI sans ajout particulier.

Reste à faire

La configuration du cron et l'ajout d'un monitoring (healthchecks, sentry ou autre...) seront fais dans une autre PR.

@alanzirek alanzirek force-pushed the import-contacts-sftp-bash branch 9 times, most recently from f2afd16 to 91449eb Compare March 11, 2025 17:58
@alanzirek alanzirek marked this pull request as ready for review March 11, 2025 20:35
@alanzirek alanzirek requested a review from Anto59290 March 11, 2025 20:35
@alanzirek alanzirek force-pushed the import-contacts-sftp-bash branch from 91449eb to 63233ad Compare March 11, 2025 21:00
Copy link
Collaborator

@Anto59290 Anto59290 left a comment

Choose a reason for hiding this comment

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

Je pense qu'on aurait a gagner a utiliser les "bash best practices" (plus de variables, peut être utiliser des choses comme nounset, pipefail, etc.)

Personnellement je n'aurais pas écrit de test sur la partie bash vue la compléxité que cela ajoute de monter le FTP, les dépendances en plus, mais je n'y vois pas plus d'inconvénients maintenant que le code est la.

return
call_command("import_contacts", "agricoll.csv")
except subprocess.CalledProcessError as e:
self.stderr.write(f"Erreur lors de l'exécution du script : {e}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

On aurrait peut être pu se passer de cela en faisant dans le cron bin/fetch_contacts_agricoll_and_key.sh && python manage.py import_contacts agricoll.csv
Si on veut garder le check des vars d'env il est possible de le faire en haut du bash, je pense que ça serait intéressant de supprimer cette commande "glue" et que chaque méthode porte ses réponsabilités (le .sh --> Je télécharge et déchiffre le CSV, la commande python --> J'ingère le CSV)

@alanzirek alanzirek force-pushed the import-contacts-sftp-bash branch 3 times, most recently from d2276f2 to f4db7ac Compare March 14, 2025 22:36
@alanzirek alanzirek force-pushed the import-contacts-sftp-bash branch from f4db7ac to e2a3553 Compare March 14, 2025 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants