Skip to content

CWE weights#110

Merged
caverav merged 19 commits intodevelopmentfrom
feature/cwe-weights
Oct 11, 2024
Merged

CWE weights#110
caverav merged 19 commits intodevelopmentfrom
feature/cwe-weights

Conversation

@Icksir
Copy link
Copy Markdown
Collaborator

@Icksir Icksir commented Sep 24, 2024

Descarga de pesos de modelo para predicción de CWE

Se adiciona en el Dockerfile ubicado en cwe_api una descarga por medio de wget de los pesos para la predicción de CWE.

Motivación y Contexto

Se necesita el modelo para hacer la recomendación de CWE.

¿Cómo ha sido probado?

Buildeé el programa sin problemas y, al testear la recomendación en vulnerabilidades (utilizando el CWE endpoint), las entrega sin problema.

Capturas de pantalla (si es apropiado):

Tipos de cambios

  • Bugfix (cambio que no interrumpe el funcionamiento y que soluciona un problema)
  • New feature (cambio que no interrumpe el funcionamiento y que añade funcionalidad)
  • Breaking change (corrección o funcionalidad que podría causar que la funcionalidad existente cambie)

Lista de verificación:

  • Mi código sigue el estilo de código de este proyecto.
  • Mi cambio requiere una modificación en la documentación.
  • He actualizado la documentación en consecuencia.
  • Requiere nuevos tests.

Summary by CodeRabbit

Resumen por CodeRabbit

  • Nuevas Funciones

    • Se ha añadido una variable de entorno FILE_ID para la configuración del entorno.
    • Se ha añadido una variable de entorno CWE_MODEL_URL para la descarga de un modelo necesario.
  • Mejoras

    • Se ha refinado el proceso de instalación de bibliotecas de PyTorch, asegurando versiones específicas.
    • Se han especificado versiones exactas para las dependencias en el archivo requirements.txt.
    • Se ha actualizado la configuración de construcción del servicio auditforge-cwe-api en docker-compose.yml, incluyendo detalles sobre el contexto y argumentos.
  • Correcciones

    • Se ha actualizado la URL utilizada en las solicitudes de clasificación para ser dinámica en lugar de estática.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 24, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

Se han realizado modificaciones en el Dockerfile para mejorar el proceso de construcción. Se ha añadido un comando apt-get update seguido de la instalación de versiones específicas de wget y unzip, y se incluye un paso de limpieza para eliminar archivos innecesarios. La instalación de las bibliotecas de PyTorch se ha refinado para incluir versiones específicas de torchvision y torchaudio antes de instalar dependencias adicionales desde requirements.txt. También se ha incorporado un nuevo comando RUN condicional para verificar la existencia del directorio modelo_cwe, descargando y descomprimiendo un archivo ZIP si no está presente. Además, se han actualizado los archivos de configuración para reflejar estas dependencias y variables de entorno.

Changes

Archivos Resumen de cambios
cwe_api/Dockerfile Se añadieron comandos para instalar wget y unzip, y para descargar y descomprimir un archivo ZIP de un modelo. Se especificaron versiones para las bibliotecas de PyTorch.
cwe_api/requirements.txt Se especificaron versiones para fastapi, uvicorn y transformers.
.env.example Se añadió la variable de entorno FILE_ID con un valor específico.
docker-compose.yml Se actualizó la configuración de construcción del servicio auditforge-cwe-api para incluir un contexto más detallado y el argumento FILE_ID.
🔗 Possibly related PRs
👥 Suggested reviewers
  • Icksir
  • jllanosg
  • Sealra

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between aab045b and cb8d954.

Files selected for processing (1)
  • cwe_api/Dockerfile (1 hunks)
Additional context used
Hadolint
cwe_api/Dockerfile

[warning] 7-7: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)


[info] 7-7: Avoid additional packages by specifying --no-install-recommends

(DL3015)


[info] 12-12: Avoid use of wget without progress bar. Use wget --progress=dot:giga <url>. Or consider using -q or -nv (shorthands for --quiet or --no-verbose).

(DL3047)

Comment thread cwe_api/Dockerfile Outdated
Comment on lines +7 to +10
RUN apt-get update \
&& apt-get install -y wget \
&& apt-get install -y unzip \
&& rm -rf /var/lib/apt/lists/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Mejora la instalación de paquetes para optimizar la imagen

El comando RUN actual funciona, pero puede mejorarse para crear una imagen Docker más eficiente y reproducible:

  1. Especifica versiones exactas para los paquetes wget y unzip.
  2. Usa la opción --no-install-recommends para reducir el tamaño de la imagen.
  3. Combina los comandos apt-get install para reducir las capas de la imagen.

Aplica este diff para implementar las mejoras:

-RUN  apt-get update \
-  && apt-get install -y wget \
-  && apt-get install -y unzip \
-  && rm -rf /var/lib/apt/lists/*
+RUN apt-get update && apt-get install -y --no-install-recommends \
+    wget=1.21.* \
+    unzip=6.0-* \
+    && rm -rf /var/lib/apt/lists/*

Asegúrate de verificar las versiones exactas de los paquetes que sean compatibles con tu sistema base.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN apt-get update \
&& apt-get install -y wget \
&& apt-get install -y unzip \
&& rm -rf /var/lib/apt/lists/*
RUN apt-get update && apt-get install -y --no-install-recommends \
wget=1.21.* \
unzip=6.0-* \
&& rm -rf /var/lib/apt/lists/*
Tools
Hadolint

[warning] 7-7: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)


[info] 7-7: Avoid additional packages by specifying --no-install-recommends

(DL3015)

Comment thread cwe_api/Dockerfile Outdated
Comment on lines +12 to +13
RUN wget "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
&& unzip modelo_cwe.zip && rm modelo_cwe.zip
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Mejora la descarga y extracción del modelo

El comando actual para descargar y extraer el modelo tiene varios problemas potenciales:

  1. No muestra el progreso de la descarga.
  2. No maneja posibles fallos en la descarga.
  3. La URL de Google Drive podría ser inestable.
  4. No especifica el destino de la extracción.

Considera aplicar estas mejoras:

-RUN wget "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
-&& unzip modelo_cwe.zip && rm modelo_cwe.zip
+RUN wget --progress=dot:giga "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
+    && unzip -d /app/models modelo_cwe.zip \
+    && rm modelo_cwe.zip \
+    || (echo "Fallo en la descarga o extracción del modelo" && exit 1)

Además, considera alojar el modelo en un servicio más estable y usar una suma de verificación para garantizar la integridad del archivo descargado.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN wget "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
&& unzip modelo_cwe.zip && rm modelo_cwe.zip
RUN wget --progress=dot:giga "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
&& unzip -d /app/models modelo_cwe.zip \
&& rm modelo_cwe.zip \
|| (echo "Fallo en la descarga o extracción del modelo" && exit 1)
Tools
Hadolint

[info] 12-12: Avoid use of wget without progress bar. Use wget --progress=dot:giga <url>. Or consider using -q or -nv (shorthands for --quiet or --no-verbose).

(DL3047)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between cb8d954 and d79acf5.

📒 Files selected for processing (1)
  • cwe_api/Dockerfile (1 hunks)
🧰 Additional context used
Hadolint
cwe_api/Dockerfile

[warning] 7-7: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)


[error] 14-14: Expected 'fi'. Fix any mentioned problems and try again.

(SC1072)

Comment thread cwe_api/Dockerfile Outdated
Comment on lines +7 to +12
RUN apt-get update \
&& apt-get install -y --no-install-recommends \
wget \
unzip \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Es necesario especificar las versiones de los paquetes

Has mejorado la instalación de paquetes utilizando --no-install-recommends, lo cual es bueno. Sin embargo, aún falta un aspecto crucial: especificar las versiones exactas de los paquetes. Esto es fundamental para garantizar la reproducibilidad y estabilidad de tu imagen Docker.

Modifica el comando RUN de la siguiente manera:

 RUN  apt-get update \
       && apt-get install -y --no-install-recommends \
-        wget \
-        unzip \
+        wget=1.21.* \
+        unzip=6.0-* \
       && apt-get clean \
       && rm -rf /var/lib/apt/lists/*

Asegúrate de verificar y utilizar las versiones específicas que sean compatibles con tu sistema base. No hacerlo podría llevar a problemas de compatibilidad en el futuro.

Committable suggestion was skipped due to low confidence.

🧰 Tools
Hadolint

[warning] 7-7: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)

Comment thread cwe_api/Dockerfile
Comment on lines +14 to +18
RUN if [ ! -d modelo_cwe ]; then \
echo "modelo_cwe not found. Downloading it..."; \
wget "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
&& unzip modelo_cwe.zip && rm modelo_cwe.zip \
fi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

La descarga del modelo requiere mejoras significativas

El proceso actual de descarga del modelo es deficiente y potencialmente problemático. Es necesario abordar varios aspectos críticos:

  1. No hay manejo de errores. Si la descarga falla, el proceso continuará como si nada hubiera pasado.
  2. No se muestra el progreso de la descarga, lo que dificulta saber si el proceso está avanzando o se ha estancado.
  3. La URL de Google Drive no es una solución estable para el almacenamiento de archivos en producción.

Implementa las siguientes mejoras de inmediato:

 RUN if [ ! -d modelo_cwe ]; then \
       echo "modelo_cwe not found. Downloading it..."; \
-      wget "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
-      && unzip modelo_cwe.zip && rm modelo_cwe.zip \
+      wget --progress=dot:giga "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
+      && unzip -d /app/models modelo_cwe.zip \
+      && rm modelo_cwe.zip \
+      || (echo "Error: Fallo en la descarga o extracción del modelo" && exit 1) \
     fi

Además, es imperativo que consideres alojar el modelo en un servicio más confiable y utilices una suma de verificación para garantizar la integridad del archivo descargado. La situación actual es inaceptable para un entorno de producción.

Committable suggestion was skipped due to low confidence.

🧰 Tools
Hadolint

[error] 14-14: Expected 'fi'. Fix any mentioned problems and try again.

(SC1072)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range comments (1)
cwe_api/Dockerfile (1)

Line range hint 1-28: Resumen de la revisión del Dockerfile

El Dockerfile tiene una estructura general adecuada, pero presenta áreas que requieren mejoras significativas:

  1. Instalación de paquetes del sistema: Bien implementada. Mantén estas buenas prácticas.

  2. Instalación de paquetes Python: Necesita mejoras menores.

    • Usa --no-cache-dir en todos los comandos pip.
    • Asegúrate de que todas las dependencias en requirements.txt tengan versiones específicas.
  3. Proceso de descarga del modelo: Requiere una revisión urgente y completa.

    • Cambia a un servicio de almacenamiento confiable y diseñado para producción.
    • Implementa una verificación de suma (checksum) para la integridad del archivo.
    • Mejora el manejo de errores y la información proporcionada en caso de fallo.

Es crucial abordar estos problemas, especialmente los relacionados con la descarga del modelo, para garantizar un entorno de producción estable y seguro. No subestimes la importancia de estas mejoras; son fundamentales para la integridad y confiabilidad de tu aplicación.

🧰 Tools
Hadolint

[warning] 14-14: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>

(DL3013)


[warning] 14-14: Avoid use of cache directory with pip. Use pip install --no-cache-dir <package>

(DL3042)


[info] 16-16: Multiple consecutive RUN instructions. Consider consolidation.

(DL3059)


[info] 18-18: Avoid use of wget without progress bar. Use wget --progress=dot:giga <url>. Or consider using -q or -nv (shorthands for --quiet or --no-verbose).

(DL3047)


[info] 18-18: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between d79acf5 and cca256e.

📒 Files selected for processing (1)
  • cwe_api/Dockerfile (1 hunks)
🧰 Additional context used
Hadolint
cwe_api/Dockerfile

[warning] 14-14: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>

(DL3013)


[warning] 14-14: Avoid use of cache directory with pip. Use pip install --no-cache-dir <package>

(DL3042)


[info] 16-16: Multiple consecutive RUN instructions. Consider consolidation.

(DL3059)


[info] 18-18: Avoid use of wget without progress bar. Use wget --progress=dot:giga <url>. Or consider using -q or -nv (shorthands for --quiet or --no-verbose).

(DL3047)


[info] 18-18: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

🔇 Additional comments not posted (1)
cwe_api/Dockerfile (1)

7-12: ¡Bien hecho con la instalación de paquetes!

La instalación de paquetes del sistema está bien implementada. Has seguido buenas prácticas al:

  1. Especificar versiones exactas para wget y unzip.
  2. Usar --no-install-recommends para reducir el tamaño de la imagen.
  3. Limpiar la caché de apt y eliminar las listas de paquetes después de la instalación.

Estas prácticas contribuyen a una imagen Docker más eficiente y reproducible. Buen trabajo.

Comment thread cwe_api/Dockerfile Outdated
Comment on lines 14 to 16
RUN pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu

RUN pip install --no-cache-dir -r requirements.txt
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Mejora necesaria en la instalación de paquetes Python

La instalación de paquetes Python requiere algunas mejoras:

  1. Falta el uso de --no-cache-dir en ambos comandos pip. Esto puede resultar en una imagen más grande de lo necesario.
  2. No se están fijando las versiones de los paquetes en el archivo requirements.txt (asumiendo por la sugerencia del análisis estático).

Aplica estas correcciones:

-RUN pip install torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu
+RUN pip install --no-cache-dir torch torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu

-RUN pip install --no-cache-dir -r requirements.txt
+RUN pip install --no-cache-dir -r requirements.txt

Además, asegúrate de que todas las dependencias en requirements.txt tengan versiones específicas. Esto es crucial para la reproducibilidad de tu entorno.

Committable suggestion was skipped due to low confidence.

🧰 Tools
Hadolint

[warning] 14-14: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>

(DL3013)


[warning] 14-14: Avoid use of cache directory with pip. Use pip install --no-cache-dir <package>

(DL3042)


[info] 16-16: Multiple consecutive RUN instructions. Consider consolidation.

(DL3059)

Comment thread cwe_api/Dockerfile
Comment on lines +18 to +23
RUN if [ ! -d modelo_cwe ]; then \
echo "modelo_cwe not found. Downloading it..."; \
wget "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
&& unzip modelo_cwe.zip && rm modelo_cwe.zip \
|| { echo "Failed to download or unzip model"; exit 1; } \
fi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Serios problemas en el proceso de descarga del modelo

El proceso actual de descarga del modelo tiene deficiencias críticas que deben abordarse de inmediato:

  1. El uso de Google Drive para almacenar archivos de producción es inaceptable. Es inestable y no está diseñado para este propósito.
  2. La descarga no muestra una barra de progreso, lo que dificulta el seguimiento de descargas grandes.
  3. El manejo de errores es básico y no proporciona información útil en caso de fallo.
  4. No se especifica el destino de extracción, lo que podría causar problemas de ubicación de archivos.

Implementa estas mejoras de inmediato:

 RUN if [ ! -d modelo_cwe ]; then \
       echo "modelo_cwe not found. Downloading it..."; \
-      wget "https://drive.usercontent.google.com/download?id=1KBoB-iLT3-2HJqMvXucMuh3Wi_r0ngP-&export=download&confirm=" -O modelo_cwe.zip \
-      && unzip modelo_cwe.zip && rm modelo_cwe.zip \
-      || { echo "Failed to download or unzip model"; exit 1; } \
+      wget --progress=dot:giga "URL_DE_UN_SERVIDOR_CONFIABLE" -O modelo_cwe.zip \
+      && unzip -d /app/models modelo_cwe.zip \
+      && rm modelo_cwe.zip \
+      || { echo "Error: Fallo en la descarga o extracción del modelo. Verifica la conexión y la URL."; exit 1; } \
     fi

Además, es imperativo que:

  1. Alojes el modelo en un servicio de almacenamiento confiable y diseñado para producción.
  2. Implementes una verificación de suma (checksum) para garantizar la integridad del archivo descargado.
  3. Consideres usar una herramienta más robusta como curl en lugar de wget para un mejor manejo de errores.

La situación actual es inaceptable para un entorno de producción y debe ser corregida con urgencia.

Committable suggestion was skipped due to low confidence.

🧰 Tools
Hadolint

[info] 18-18: Avoid use of wget without progress bar. Use wget --progress=dot:giga <url>. Or consider using -q or -nv (shorthands for --quiet or --no-verbose).

(DL3047)


[info] 18-18: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range comments (1)
cwe_api/Dockerfile (1)

Line range hint 1-5: Optimiza las partes no modificadas del Dockerfile

Aunque estas secciones no han cambiado, hay oportunidades para mejorar la eficiencia y seguridad del Dockerfile:

  1. Usa una imagen base más específica:

    -FROM python:slim
    +FROM python:3.9-slim-buster

    Especifica la versión exacta de Python para mayor consistencia.

  2. Limita los archivos copiados:

    -COPY . .
    +COPY requirements.txt .
    +COPY app/ ./app/

    Copia solo los archivos necesarios para reducir el tamaño de la imagen y evitar incluir archivos sensibles accidentalmente.

  3. Crea un usuario no root:

    WORKDIR /app
    +RUN useradd -m myuser
    +USER myuser

    Ejecutar la aplicación como usuario no root mejora la seguridad.

  4. Considera usar un archivo .dockerignore para excluir archivos innecesarios durante la construcción de la imagen.

Implementa estos cambios para mejorar significativamente la eficiencia y seguridad de tu Dockerfile.

Also applies to: 25-29

🧰 Tools
Hadolint

[warning] 14-14: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>

(DL3013)


[info] 16-16: Multiple consecutive RUN instructions. Consider consolidation.

(DL3059)


[info] 18-18: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between cca256e and c8fe36b.

📒 Files selected for processing (1)
  • cwe_api/Dockerfile (1 hunks)
🧰 Additional context used
Hadolint
cwe_api/Dockerfile

[warning] 14-14: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>

(DL3013)


[info] 16-16: Multiple consecutive RUN instructions. Consider consolidation.

(DL3059)


[info] 18-18: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

Comment thread cwe_api/Dockerfile Outdated
Comment on lines +7 to +12
RUN apt-get update \
&& apt-get install -y --no-install-recommends \
wget=1.21.* \
unzip=6.0-* \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Mejora la legibilidad de la instalación de paquetes

La instalación de paquetes está bien optimizada, pero podemos mejorar su legibilidad y mantenibilidad.

Aplica este cambio para mejorar la estructura del comando:

 RUN  apt-get update \
-      && apt-get install -y --no-install-recommends \
-        wget=1.21.* \
-        unzip=6.0-* \
-      && apt-get clean \
-      && rm -rf /var/lib/apt/lists/*
+    && apt-get install -y --no-install-recommends \
+       wget=1.21.* \
+       unzip=6.0-* \
+    && apt-get clean \
+    && rm -rf /var/lib/apt/lists/*

Esto mejora la consistencia en la indentación y hace que el comando sea más fácil de leer y mantener.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN apt-get update \
&& apt-get install -y --no-install-recommends \
wget=1.21.* \
unzip=6.0-* \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*
RUN apt-get update \
&& apt-get install -y --no-install-recommends \
wget=1.21.* \
unzip=6.0-* \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*

Comment thread cwe_api/Dockerfile Outdated
Comment on lines 14 to 16
RUN pip install --no-cache-dir torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu

RUN pip install --no-cache-dir -r requirements.txt
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Optimiza la instalación de paquetes Python

La instalación de paquetes Python tiene margen de mejora en cuanto a la especificación de versiones y la optimización del Dockerfile.

  1. No se están especificando versiones para los paquetes torch, lo que puede llevar a problemas de reproducibilidad.
  2. Hay múltiples instrucciones RUN consecutivas, lo que aumenta innecesariamente el número de capas de la imagen.

Aplica estos cambios para abordar estos problemas:

-RUN pip install --no-cache-dir torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu
-
-RUN pip install --no-cache-dir -r requirements.txt
+RUN pip install --no-cache-dir torchvision==0.15.2 torchaudio==2.0.2 --index-url https://download.pytorch.org/whl/cpu \
+    && pip install --no-cache-dir -r requirements.txt

Asegúrate de especificar las versiones correctas para torchvision y torchaudio que sean compatibles con tu proyecto. Además, verifica que todas las dependencias en requirements.txt tengan versiones específicas.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN pip install --no-cache-dir torchvision torchaudio --index-url https://download.pytorch.org/whl/cpu
RUN pip install --no-cache-dir -r requirements.txt
RUN pip install --no-cache-dir torchvision==0.15.2 torchaudio==2.0.2 --index-url https://download.pytorch.org/whl/cpu \
&& pip install --no-cache-dir -r requirements.txt
🧰 Tools
Hadolint

[warning] 14-14: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>

(DL3013)


[info] 16-16: Multiple consecutive RUN instructions. Consider consolidation.

(DL3059)

Comment thread cwe_api/Dockerfile
@jllanosg
Copy link
Copy Markdown
Collaborator

jllanosg commented Oct 1, 2024

@Icksir debes hacerte cargo de los comentarios que deja coderabbit (lo cerré sin querer)

@jllanosg jllanosg closed this Oct 1, 2024
@jllanosg jllanosg reopened this Oct 1, 2024
@sealra
Copy link
Copy Markdown
Collaborator

sealra commented Oct 5, 2024

Por favor corregir los comentarios de coderabbit, por el resto LGTM 👍

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between c8fe36b and 3ff7b8b.

📒 Files selected for processing (1)
  • cwe_api/Dockerfile (1 hunks)
🧰 Additional context used
🪛 Hadolint
cwe_api/Dockerfile

[info] 17-17: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

🔇 Additional comments (2)
cwe_api/Dockerfile (2)

17-22: ⚠️ Potential issue

Graves deficiencias en el proceso de descarga del modelo

El proceso actual de descarga del modelo es completamente inaceptable para un entorno de producción. Los problemas críticos incluyen:

  1. El uso de Google Drive para almacenar archivos de producción es absolutamente inadecuado y poco fiable.
  2. No hay verificación de la integridad del archivo descargado.
  3. El manejo de errores es rudimentario y no proporciona información útil para la depuración.
  4. No se especifica el destino de extracción, lo que podría causar problemas de ubicación de archivos.

Es imperativo que implementes las siguientes mejoras de inmediato:

  1. Migra el modelo a un servicio de almacenamiento confiable diseñado para producción.
  2. Implementa una verificación de suma (checksum) para garantizar la integridad del archivo descargado.
  3. Mejora el manejo de errores y la indicación de progreso.
  4. Especifica claramente el destino de extracción.

Aplica estos cambios urgentemente:

 RUN if [ ! -d modelo_cwe ]; then \
       echo "modelo_cwe not found. Downloading it..."; \
-      wget --progress=bar:force "https://drive.usercontent.google.com/download?id=1OtRNObv-Il2B5nDnpzMSGj_yBJAlskuS&export=download&confirm=" -O modelo_cwe.zip \
-      && unzip modelo_cwe.zip && rm modelo_cwe.zip \
-      || { echo "Failed to download or unzip model"; exit 1; } \
+      wget --progress=bar:force "https://tu-servidor-confiable.com/path/to/modelo_cwe.zip" -O modelo_cwe.zip \
+      && echo "checksum-esperado modelo_cwe.zip" | sha256sum -c \
+      && unzip -d /app/models modelo_cwe.zip \
+      && rm modelo_cwe.zip \
+      || { echo "Error crítico: Fallo en la descarga, verificación o extracción del modelo. Verifica la conexión, la integridad del archivo y los permisos."; exit 1; } \
     fi

Reemplaza "https://tu-servidor-confiable.com/path/to/modelo_cwe.zip" con la URL real de tu servicio de almacenamiento y "checksum-esperado" con el checksum correcto del archivo. Esta situación es completamente inaceptable para un entorno de producción y debe ser corregida con la máxima urgencia.

#!/bin/bash
# Verificar que la URL del modelo no sea de Google Drive
if grep -q "drive.google.com\|drive.usercontent.google.com" Dockerfile; then
  echo "Error crítico: Se detectó una URL de Google Drive en el Dockerfile"
  exit 1
else
  echo "No se encontraron URLs de Google Drive en el Dockerfile"
fi
🧰 Tools
🪛 Hadolint

[info] 17-17: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)


14-15: ⚠️ Potential issue

Optimización de la instalación de dependencias Python necesaria

La instalación actual de dependencias Python tiene deficiencias que deben abordarse:

  1. No se está especificando la versión de torch, lo cual es crítico para la reproducibilidad del entorno.
  2. No hay garantía de que las dependencias en requirements.txt tengan versiones fijas.

Es imperativo que implementes las siguientes mejoras:

  1. Especifica la versión exacta de torch compatible con las versiones de torchvision y torchaudio.
  2. Asegúrate de que todas las dependencias en requirements.txt tengan versiones fijas.
  3. Añade un paso de verificación para confirmar que todas las dependencias se han instalado correctamente.

Aplica estos cambios inmediatamente:

-RUN pip install --no-cache-dir torchvision==0.15.2 torchaudio==2.0.2 --index-url https://download.pytorch.org/whl/cpu \
-    && pip install --no-cache-dir -r requirements.txt
+RUN pip install --no-cache-dir torch==2.0.1 torchvision==0.15.2 torchaudio==2.0.2 --index-url https://download.pytorch.org/whl/cpu \
+    && pip install --no-cache-dir -r requirements.txt \
+    && pip check \
+    || (echo "Error: Fallo en la instalación o verificación de dependencias Python" && exit 1)

Además, revisa y actualiza el archivo requirements.txt para asegurar que todas las dependencias tengan versiones específicas. La situación actual es inaceptable para un entorno de producción y debe ser corregida con la máxima urgencia.

Comment thread cwe_api/Dockerfile Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 3ff7b8b and ba58f23.

📒 Files selected for processing (1)
  • cwe_api/Dockerfile (1 hunks)
🧰 Additional context used
🪛 Hadolint
cwe_api/Dockerfile

[info] 7-7: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)


[info] 18-18: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

Comment thread cwe_api/Dockerfile Outdated
Comment thread cwe_api/Dockerfile Outdated
Comment thread cwe_api/Dockerfile
sealra
sealra previously approved these changes Oct 5, 2024
Copy link
Copy Markdown
Collaborator

@sealra sealra left a comment

Choose a reason for hiding this comment

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

LGTM 👍, asigné a los demás como reviewers también, que no estaban asignados.

@jllanosg
Copy link
Copy Markdown
Collaborator

jllanosg commented Oct 7, 2024

@Icksir debes hacerte cargo de los comentarios que deja coderabbit

@Icksir
Copy link
Copy Markdown
Collaborator Author

Icksir commented Oct 7, 2024

@Icksir debes hacerte cargo de los comentarios que deja coderabbit

Ya lo hice, el que no pesqué fue el que decía que debía colocarlo en otro lugar que no fuera Drive

@caverav
Copy link
Copy Markdown
Owner

caverav commented Oct 7, 2024

El PR dice que hay que actualizar documentación pero no se ha actualizado

@Icksir
Copy link
Copy Markdown
Collaborator Author

Icksir commented Oct 7, 2024

El PR dice que hay que actualizar documentación pero no se ha actualizado

Ahí lo cambié, lo había marcado cuando pensaba que se debía hacer algún paso adicional para descargar los pesos (fuera del docker-compose up). Ahora no requiere cambio a la documentación.

Comment thread cwe_api/Dockerfile Outdated

RUN if [ ! -d modelo_cwe ]; then \
echo "modelo_cwe not found. Downloading it..."; \
wget --progress=bar:force "https://drive.usercontent.google.com/download?id=1OtRNObv-Il2B5nDnpzMSGj_yBJAlskuS&export=download&confirm=" -O modelo_cwe.zip \
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

esto no podría manejarse con una variable de entorno?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

qué cosa?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Considero que no es la mejor opción el manejarlo con una variable de entorno, ya que es algo menos eficiente en muchos casos. El código referenciado revisa automáticamente la existencia del directorio durante el buildeo

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

La URL del drive

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Honestamente no sé si hace una diferencia o no

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@iTzGooDLife no se si tu comentario iba en linea con lo que me referia (tomar la url del drive de un .env);

@Icksir si la hace, ya que si cambia la url del archivo por cualquier motivo, se debe hacer un PR para modificar esa URL; en cambio si la url viene en un .env, solo se debe cambiar ahí. Esto sería util en el caso actual, ya que si solo tuvieramos una url fija tipo "auditforge.com/modelo.zip" nunca habría que modificarla

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yo creo que debería mantenerse así, la url es invariable y que se caiga o se modifique el link de drive es poco probable. Encuentro que es más sencillo sin .env

Copy link
Copy Markdown
Collaborator

@jllanosg jllanosg Oct 9, 2024

Choose a reason for hiding this comment

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

Yo creo que debería mantenerse así, la url es invariable y que se caiga o se modifique el link de drive es poco probable. Encuentro que es más sencillo sin .env

No que tenemos que cambiar el modelo proximamente? En ese caso si cambiaría la url no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

El de CWE ya es el definitivo, recabé los datos actualizados y ya está entrenado

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between ba58f23 and 3890d53.

📒 Files selected for processing (2)
  • cwe_api/Dockerfile (1 hunks)
  • cwe_api/requirements.txt (1 hunks)
🧰 Additional context used
🪛 Hadolint
cwe_api/Dockerfile

[info] 7-7: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)


[info] 19-19: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

🔇 Additional comments (3)
cwe_api/Dockerfile (3)

7-14: La gestión de errores es deficiente y debe mejorarse

El uso de set -e es un buen comienzo, pero la estructura actual del comando podría no manejar correctamente todos los escenarios de error. El bloque || { ... } al final solo se ejecutará si el último comando falla, no si alguno de los comandos intermedios falla.

Modifica el comando de la siguiente manera para garantizar una mejor gestión de errores:

 RUN set -e \
-    && apt-get update -y \
-    && apt-get install -y --no-install-recommends \
-       wget=1.21.3-1+deb11u1 \
-       unzip=6.0-26+deb11u1 \
-    && apt-get clean \
-    && rm -rf /var/lib/apt/lists/* \
-    || { echo "Error: Fallo en la instalación de paquetes"; exit 1; }
+    && (apt-get update -y \
+        && apt-get install -y --no-install-recommends \
+           wget=1.21.3-1+deb11u1 \
+           unzip=6.0-26+deb11u1 \
+        && apt-get clean \
+        && rm -rf /var/lib/apt/lists/*) \
+    || { echo "Error: Fallo en la instalación de paquetes"; exit 1; }

Esta modificación asegura que cualquier error en el proceso de instalación se maneje adecuadamente y detenga la construcción de la imagen.

🧰 Tools
🪛 Hadolint

[info] 7-7: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)


16-17: Las versiones en requirements.txt deben estar especificadas

Aunque has fijado correctamente las versiones de torchvision y torchaudio, es crucial que todas las dependencias en requirements.txt también tengan versiones específicas. No hacerlo puede llevar a inconsistencias y problemas de reproducibilidad en diferentes entornos.

Modifica inmediatamente tu archivo requirements.txt para incluir versiones exactas de todas las dependencias. Por ejemplo:

package1==1.2.3
package2==2.3.4

Esta práctica es esencial para garantizar la consistencia y la reproducibilidad de tu entorno de desarrollo y producción. No hacerlo es una negligencia que puede causar problemas graves en el futuro.


19-24: El proceso de descarga del modelo es inaceptable y debe ser corregido inmediatamente

El método actual de descarga del modelo tiene deficiencias críticas que comprometen la integridad y fiabilidad de tu aplicación:

  1. El uso de Google Drive para archivos de producción es totalmente inadecuado y poco fiable.
  2. No hay verificación de la integridad del archivo descargado.
  3. La gestión de errores es insuficiente y podría no detectar todos los problemas potenciales.

Es imperativo que implementes las siguientes mejoras de inmediato:

  1. Aloja el modelo en un servicio de almacenamiento confiable diseñado para producción.
  2. Implementa una verificación de suma (checksum) para garantizar la integridad del archivo.
  3. Mejora la gestión de errores para cubrir todos los escenarios posibles.

Aplica estos cambios como punto de partida:

-RUN if [ ! -d modelo_cwe ]; then \
-      echo "modelo_cwe not found. Downloading it..."; \
-      wget --progress=bar:force "https://drive.usercontent.google.com/download?id=1OtRNObv-Il2B5nDnpzMSGj_yBJAlskuS&export=download&confirm=" -O modelo_cwe.zip \
-      && unzip modelo_cwe.zip && rm modelo_cwe.zip \
-      || { echo "Failed to download or unzip model"; exit 1; } \
-    fi
+RUN set -e \
+    && if [ ! -d modelo_cwe ]; then \
+        echo "modelo_cwe no encontrado. Descargando..."; \
+        wget --progress=bar:force "URL_DE_TU_SERVIDOR_CONFIABLE" -O modelo_cwe.zip \
+        && echo "CHECKSUM_ESPERADO modelo_cwe.zip" | sha256sum -c \
+        && unzip -d /app/models modelo_cwe.zip \
+        && rm modelo_cwe.zip \
+        || { echo "Error: Fallo en la descarga, verificación o extracción del modelo."; exit 1; } \
+    fi

Reemplaza "URL_DE_TU_SERVIDOR_CONFIABLE" con la URL real de tu servicio de almacenamiento y "CHECKSUM_ESPERADO" con el checksum correcto del archivo. Esta situación es inaceptable para un entorno de producción y debe ser corregida con la máxima urgencia.

🧰 Tools
🪛 Hadolint

[info] 19-19: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

Comment thread cwe_api/requirements.txt
Copy link
Copy Markdown
Collaborator

@jllanosg jllanosg left a comment

Choose a reason for hiding this comment

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

No me buildea la app:

=> ERROR [auditforge-cwe-api 4/6] RUN set -e     && apt-get update -y     && apt-get install -y --no-install-recommends        wget=1.21.3-1+deb11u1        unzip=6.0-26+deb11u1     && apt-get clean     &  5.4s
------                                                                                                                                                                                                             
 > [auditforge-cwe-api 4/6] RUN set -e     && apt-get update -y     && apt-get install -y --no-install-recommends        wget=1.21.3-1+deb11u1        unzip=6.0-26+deb11u1     && apt-get clean     && rm -rf /var/lib/apt/lists/*     || { echo "Error: Fallo en la instalación de paquetes"; exit 1; }:                                                                                                                             
0.354 Get:1 http://deb.debian.org/debian bookworm InRelease [151 kB]                                                                                                                                               
0.395 Get:2 http://deb.debian.org/debian bookworm-updates InRelease [55.4 kB]                                                                                                                                      
0.397 Get:3 http://deb.debian.org/debian-security bookworm-security InRelease [48.0 kB]                                                                                                                            
0.540 Get:4 http://deb.debian.org/debian bookworm/main amd64 Packages [8787 kB]                                                                                                                                    
0.804 Get:5 http://deb.debian.org/debian bookworm-updates/main amd64 Packages [2468 B]                                                                                                                             
0.808 Get:6 http://deb.debian.org/debian-security bookworm-security/main amd64 Packages [188 kB]
2.735 Fetched 9232 kB in 2s (3784 kB/s)
2.735 Reading package lists...
4.088 Reading package lists...
5.102 Building dependency tree...
5.312 Reading state information...
5.323 Package wget is not available, but is referred to by another package.
5.323 This may mean that the package is missing, has been obsoleted, or
5.323 is only available from another source
5.323 
5.323 Package unzip is not available, but is referred to by another package.
5.323 This may mean that the package is missing, has been obsoleted, or
5.323 is only available from another source
5.323 
5.331 E: Version '1.21.3-1+deb11u1' for 'wget' was not found
5.331 E: Version '6.0-26+deb11u1' for 'unzip' was not found
5.332 Error: Fallo en la instalación de paquetes
------
failed to solve: process "/bin/sh -c set -e     && apt-get update -y     && apt-get install -y --no-install-recommends        wget=1.21.3-1+deb11u1        unzip=6.0-26+deb11u1     && apt-get clean     && rm -rf /var/lib/apt/lists/*     || { echo \"Error: Fallo en la instalación de paquetes\"; exit 1; }" did not complete successfully: exit code: 1```

Comment thread cwe_api/requirements.txt
@Icksir Icksir dismissed stale reviews from sealra and iTzGooDLife via 6ae118b October 10, 2024 13:30
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 3890d53 and 6ae118b.

📒 Files selected for processing (3)
  • .env.example (1 hunks)
  • cwe_api/Dockerfile (1 hunks)
  • docker-compose.yml (1 hunks)
🧰 Additional context used
🪛 Hadolint
cwe_api/Dockerfile

[warning] 8-8: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)


[info] 8-8: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)


[info] 20-20: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

🪛 yamllint
docker-compose.yml

[error] 51-51: trailing spaces

(trailing-spaces)

🔇 Additional comments (2)
cwe_api/Dockerfile (1)

17-18: Verifique las versiones en requirements.txt

La instalación de paquetes de Python está bien estructurada, con versiones específicas para torchvision y torchaudio. Sin embargo, no podemos ver el contenido de requirements.txt.

Es crucial que todas las dependencias en requirements.txt también tengan versiones específicas. Si no es así, modifique inmediatamente su archivo requirements.txt para incluir versiones exactas de todas las dependencias. Por ejemplo:

package1==1.2.3
package2==2.3.4

Esta práctica es esencial para garantizar la consistencia y la reproducibilidad de su entorno de desarrollo y producción.

docker-compose.yml (1)

47-51: Los cambios en la configuración de construcción son apropiados.

La nueva configuración de construcción para el servicio auditforge-cwe-api proporciona una mayor especificidad, lo cual es una buena práctica. La introducción del argumento FILE_ID sugiere que el Dockerfile ahora requiere este valor durante el proceso de construcción, lo que se alinea con los objetivos del PR de añadir una característica de descarga de pesos del modelo.

🧰 Tools
🪛 yamllint

[error] 51-51: trailing spaces

(trailing-spaces)

Comment thread .env.example Outdated
Comment thread cwe_api/Dockerfile
Comment thread cwe_api/Dockerfile
Comment thread cwe_api/Dockerfile
Comment thread docker-compose.yml Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@Icksir
Copy link
Copy Markdown
Collaborator Author

Icksir commented Oct 10, 2024

No me buildea la app:

=> ERROR [auditforge-cwe-api 4/6] RUN set -e     && apt-get update -y     && apt-get install -y --no-install-recommends        wget=1.21.3-1+deb11u1        unzip=6.0-26+deb11u1     && apt-get clean     &  5.4s
------                                                                                                                                                                                                             
 > [auditforge-cwe-api 4/6] RUN set -e     && apt-get update -y     && apt-get install -y --no-install-recommends        wget=1.21.3-1+deb11u1        unzip=6.0-26+deb11u1     && apt-get clean     && rm -rf /var/lib/apt/lists/*     || { echo "Error: Fallo en la instalación de paquetes"; exit 1; }:                                                                                                                             
0.354 Get:1 http://deb.debian.org/debian bookworm InRelease [151 kB]                                                                                                                                               
0.395 Get:2 http://deb.debian.org/debian bookworm-updates InRelease [55.4 kB]                                                                                                                                      
0.397 Get:3 http://deb.debian.org/debian-security bookworm-security InRelease [48.0 kB]                                                                                                                            
0.540 Get:4 http://deb.debian.org/debian bookworm/main amd64 Packages [8787 kB]                                                                                                                                    
0.804 Get:5 http://deb.debian.org/debian bookworm-updates/main amd64 Packages [2468 B]                                                                                                                             
0.808 Get:6 http://deb.debian.org/debian-security bookworm-security/main amd64 Packages [188 kB]
2.735 Fetched 9232 kB in 2s (3784 kB/s)
2.735 Reading package lists...
4.088 Reading package lists...
5.102 Building dependency tree...
5.312 Reading state information...
5.323 Package wget is not available, but is referred to by another package.
5.323 This may mean that the package is missing, has been obsoleted, or
5.323 is only available from another source
5.323 
5.323 Package unzip is not available, but is referred to by another package.
5.323 This may mean that the package is missing, has been obsoleted, or
5.323 is only available from another source
5.323 
5.331 E: Version '1.21.3-1+deb11u1' for 'wget' was not found
5.331 E: Version '6.0-26+deb11u1' for 'unzip' was not found
5.332 Error: Fallo en la instalación de paquetes
------
failed to solve: process "/bin/sh -c set -e     && apt-get update -y     && apt-get install -y --no-install-recommends        wget=1.21.3-1+deb11u1        unzip=6.0-26+deb11u1     && apt-get clean     && rm -rf /var/lib/apt/lists/*     || { echo \"Error: Fallo en la instalación de paquetes\"; exit 1; }" did not complete successfully: exit code: 1```

Sí, taba leseando por las versiones del requirement y algunos del apt-get, así que las saqué

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 6ae118b and 47cecff.

📒 Files selected for processing (1)
  • docker-compose.yml (1 hunks)
🧰 Additional context used

Comment thread docker-compose.yml Outdated
Comment thread cwe_api/Dockerfile
Comment thread cwe_api/Dockerfile
Copy link
Copy Markdown
Collaborator

@jllanosg jllanosg left a comment

Choose a reason for hiding this comment

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

Generalizar a cualquier sitio de descarga

Comment thread cwe_api/Dockerfile Outdated
Comment thread docker-compose.yml Outdated
Comment thread .env.example Outdated
Icksir and others added 4 commits October 10, 2024 11:16
Co-authored-by: José Llanos <74331945+jllanosg@users.noreply.github.com>
Co-authored-by: José Llanos <74331945+jllanosg@users.noreply.github.com>
Co-authored-by: José Llanos <74331945+jllanosg@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Outside diff range comments (1)
cwe_api/Dockerfile (1)

Line range hint 1-30: Revisión general del Dockerfile

Aunque la estructura general del Dockerfile es buena, hay algunas áreas que requieren atención:

  1. El ARG FILE_ID se define pero no se utiliza. Elimínelo si no es necesario o asegúrese de utilizarlo correctamente.

  2. La instrucción COPY al principio es demasiado amplia. Sea más específico sobre qué archivos necesita copiar al contenedor.

  3. Considere usar multi-stage builds para reducir el tamaño final de la imagen, especialmente si hay herramientas de compilación que no son necesarias en la imagen final.

  4. Asegúrese de que todas las dependencias en requirements.txt tengan versiones específicas para garantizar la reproducibilidad de la construcción.

Aborde estos puntos para mejorar la eficiencia y mantenibilidad de su Dockerfile.

🧰 Tools
🪛 Hadolint

[warning] 8-8: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)


[info] 8-8: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)


[info] 20-20: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 47cecff and 4727351.

📒 Files selected for processing (3)
  • .env.example (1 hunks)
  • cwe_api/Dockerfile (1 hunks)
  • docker-compose.yml (1 hunks)
🧰 Additional context used
🪛 Hadolint
cwe_api/Dockerfile

[warning] 8-8: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)


[info] 8-8: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)


[info] 20-20: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

🔇 Additional comments (1)
cwe_api/Dockerfile (1)

17-18: Instalación de paquetes Python correcta

La instalación de paquetes Python está bien implementada. Ha especificado versiones para torchvision y torchaudio, y utiliza el flag --no-cache-dir para reducir el tamaño de la imagen. La instalación de requisitos adicionales desde requirements.txt también es apropiada.

Comment thread .env.example
@@ -0,0 +1 @@
CWE_MODEL_URL=https://drive.usercontent.google.com/download?id=1OtRNObv-Il2B5nDnpzMSGj_yBJAlskuS&export=download&confirm=``` No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

La URL del modelo CWE presenta problemas de seguridad y mantenibilidad.

El uso de una URL directa de Google Drive para descargar el modelo CWE no es una solución óptima. Presenta los siguientes problemas:

  1. Seguridad: La URL contiene un ID de archivo específico que podría ser información sensible.
  2. Mantenibilidad: Si el archivo en Google Drive cambia, esta URL tendrá que ser actualizada manualmente.
  3. Fiabilidad: Depender de un servicio de terceros (Google Drive) para un componente crítico puede causar problemas si el servicio no está disponible.

Se recomienda encarecidamente considerar las siguientes alternativas:

  1. Alojar el modelo en un servicio de almacenamiento más apropiado para este tipo de archivos, como Amazon S3 o Google Cloud Storage.
  2. Implementar un sistema de versionado para el modelo, permitiendo actualizaciones más controladas.
  3. Incluir el modelo directamente en el repositorio si su tamaño lo permite, o utilizar Git LFS para archivos grandes.

Estas alternativas mejorarían significativamente la seguridad, mantenibilidad y fiabilidad de la aplicación. ¿Necesita ayuda para implementar alguna de estas soluciones?

Comment thread cwe_api/Dockerfile
Comment on lines 5 to +6
COPY . .
ARG FILE_ID
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

La instrucción COPY es demasiado amplia y el ARG FILE_ID no se utiliza

La instrucción COPY . . copia todo el contenido del contexto de construcción al contenedor. Esto puede incluir archivos innecesarios, aumentando el tamaño de la imagen. Sea más específico con lo que se copia.

Además, se ha añadido un nuevo ARG FILE_ID que no se utiliza en ninguna parte visible del Dockerfile. Si no es necesario, elimínelo. Si es necesario, asegúrese de utilizarlo correctamente y documentar su propósito.

Comment thread cwe_api/Dockerfile
Comment on lines +8 to +15
RUN set -e \
&& apt-get update -y \
&& apt-get install -y --no-install-recommends \
wget \
unzip \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/* \
|| { echo "Error: Fallo en la instalación de paquetes"; exit 1; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Es necesario especificar las versiones de los paquetes

No ha especificado las versiones de los paquetes wget y unzip. Esto puede llevar a inconsistencias entre diferentes construcciones de la imagen. Modifique la instalación de paquetes para incluir versiones específicas:

RUN set -e \
    && apt-get update -y \
    && apt-get install -y --no-install-recommends \
       wget=1.21.* \
       unzip=6.0-* \
    && apt-get clean \
    && rm -rf /var/lib/apt/lists/* \
    || { echo "Error: Fallo en la instalación de paquetes"; exit 1; }

Asegúrese de verificar y utilizar las versiones específicas que sean compatibles con su sistema base.

🧰 Tools
🪛 Hadolint

[warning] 8-8: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)


[info] 8-8: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

Comment thread cwe_api/Dockerfile
Comment on lines +20 to +25
RUN if [ ! -d modelo_cwe ]; then \
echo "modelo_cwe not found. Downloading it..."; \
wget --progress=bar:force "$CWE_MODEL_URL" -O modelo_cwe.zip \
&& unzip modelo_cwe.zip && rm modelo_cwe.zip \
|| { echo "Failed to download or unzip model"; exit 1; } \
fi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

El proceso de descarga del modelo requiere mejoras significativas

Aunque entiendo que el uso de Google Drive es temporal, este método tiene deficiencias críticas:

  1. No hay verificación de la integridad del archivo descargado.
  2. La gestión de errores podría ser más robusta.
  3. No se especifica un directorio de destino para la extracción.

Implemente las siguientes mejoras:

  1. Añada una verificación de suma (checksum) para garantizar la integridad del archivo.
  2. Mejore la gestión de errores para cubrir más escenarios.
  3. Especifique un directorio de destino para la extracción.

Ejemplo de mejora:

RUN if [ ! -d modelo_cwe ]; then \
      echo "modelo_cwe not found. Downloading it..."; \
      wget --progress=bar:force "$CWE_MODEL_URL" -O modelo_cwe.zip \
      && echo "${CHECKSUM} modelo_cwe.zip" | sha256sum -c \
      && unzip -d /app/models modelo_cwe.zip \
      && rm modelo_cwe.zip \
      || { echo "Error: Fallo en la descarga, verificación o extracción del modelo"; exit 1; } \
    fi

Asegúrese de definir CHECKSUM como un ARG o ENV en el Dockerfile.

🧰 Tools
🪛 Hadolint

[info] 20-20: Note that A && B || C is not if-then-else. C may run when A is true.

(SC2015)

Comment thread docker-compose.yml
Comment on lines +47 to +51
build:
context: ./cwe_api
dockerfile: Dockerfile
args:
CWE_MODEL_URL: ${CWE_MODEL_URL}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

CWE_MODEL_URL no está definido en el entorno actual

El script de verificación ha confirmado que la variable de entorno CWE_MODEL_URL no está definida en el entorno actual. Aunque está especificada en .env.example, es esencial que CWE_MODEL_URL esté definida para evitar fallos en la construcción del servicio auditforge-cwe-api.

Te recomiendo realizar las siguientes acciones:

  1. Define CWE_MODEL_URL en tu entorno actual o en un archivo de configuración adecuado.
  2. Asegúrate de que cualquier miembro del equipo actualice sus entornos con esta variable.
  3. Considera agregar validaciones adicionales en el proceso de construcción para asegurar que todas las variables de entorno necesarias estén definidas.
🔗 Analysis chain

La estructura de construcción ha mejorado, pero hay un problema potencial con CWE_MODEL_URL

La nueva estructura de construcción para el servicio auditforge-cwe-api es más clara y flexible. Sin embargo, hay un problema potencial con el argumento CWE_MODEL_URL:

  1. No se proporciona un valor predeterminado para CWE_MODEL_URL.
  2. No hay garantía de que CWE_MODEL_URL esté definido en el entorno.

Esto podría causar fallos en la construcción si la variable de entorno no está configurada.

Te exijo que hagas lo siguiente:

  1. Asegúrate de que CWE_MODEL_URL esté definido en tu entorno o en un archivo .env.
  2. Añade un valor predeterminado para CWE_MODEL_URL en caso de que no esté definido:
 build:
   context: ./cwe_api
   dockerfile: Dockerfile
   args:
-    CWE_MODEL_URL: ${CWE_MODEL_URL}
+    CWE_MODEL_URL: ${CWE_MODEL_URL:-URL_POR_DEFECTO}

Reemplaza "URL_POR_DEFECTO" con una URL apropiada para tu caso de uso.

  1. Documenta esta nueva variable de entorno en el README del proyecto o en la documentación relevante.

Ejecuta el siguiente script para verificar la configuración de la variable de entorno:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Descripción: Verifica la configuración de la variable de entorno CWE_MODEL_URL

# Prueba: Buscar la definición de CWE_MODEL_URL en archivos .env o similares
echo "Buscando la definición de CWE_MODEL_URL en archivos de configuración:"
rg -i 'CWE_MODEL_URL\s*=' .env* *env* config* 

# Prueba: Verificar si CWE_MODEL_URL está definido en el entorno actual
echo "Verificando si CWE_MODEL_URL está definido en el entorno actual:"
if [ -z "${CWE_MODEL_URL}" ]; then
  echo "ADVERTENCIA: CWE_MODEL_URL no está definido en el entorno actual."
else
  echo "CWE_MODEL_URL está definido en el entorno actual."
fi

Length of output: 735

Copy link
Copy Markdown
Collaborator

@jllanosg jllanosg left a comment

Choose a reason for hiding this comment

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

Funciona

Copy link
Copy Markdown
Collaborator

@sealra sealra left a comment

Choose a reason for hiding this comment

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

LGTM

@caverav caverav merged commit 8551af3 into development Oct 11, 2024
@caverav caverav deleted the feature/cwe-weights branch October 11, 2024 01:47
@coderabbitai coderabbitai Bot mentioned this pull request Oct 11, 2024
7 tasks
@coderabbitai coderabbitai Bot mentioned this pull request Nov 3, 2024
7 tasks
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.

5 participants