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

Documentation Improvement: Adding Detailed Steps for Implementing a Simple Linter and Warnings for Actions to Avoid #2399

Closed
ThomasSanson opened this issue Feb 27, 2023 · 23 comments
Labels
enhancement New feature or request O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity

Comments

@ThomasSanson
Copy link
Contributor

I would like to propose an improvement to the existing documentation by adding detailed steps for implementing a simple linter and important warnings for actions to avoid.

I suggest adding detailed steps for implementing an existing simple linter, explaining each step in detail so that users can easily understand and follow the instructions. Additionally, I propose implementing clear warnings for actions to avoid, such as touching generated files created by the linter.

I am willing to contribute to writing these instructions and assisting with the implementation of these warnings.

Thank you in advance for considering my proposal.

@ThomasSanson ThomasSanson added the enhancement New feature or request label Feb 27, 2023
@bdovaz
Copy link
Collaborator

bdovaz commented Feb 27, 2023

Having this reference: https://megalinter.io/beta/contributing/#add-a-new-linter

Points that come to mind:

  • Clearly indicate where to create the descriptor.
  • Explain the automatic testing system that generates a class for each linter with several methods (success, fail, get help, get version, sarif, ...):
    • Skips certain tests based on certain fields in the linter or certain conditions.
    • The magic of those "_good", "_bad" and "_fix" in folder or filenames
    • Indicate that input files must be created for each test that requires them (success, fail, fix).

I personally added these 2 blocks that I understand that they make development much easier and at the time I did not have this information:

https://megalinter.io/beta/contributing/#execute-the-tests-locally-visual-studio-code

https://megalinter.io/beta/contributing/#execute-linter-tests-inside-the-container

It helps you not to depend so much on CI and those eternal executions that frustrate a lot.

If you see something that can be improved, you can say it.

@nvuillam
Copy link
Member

Maybe we could split the contributing into several pages ?
It would maybe look less messy ^^

@ThomasSanson
Copy link
Contributor Author

test.sh

#!/bin/bash

name="${1:-kubernetes_helm}"
image_name="oxsecurity/megalinter-only-${name}"

# Build and tag the Docker image
docker buildx build -f "linters/${name}/Dockerfile" . --tag "${image_name}"

# Use the case substitution to get the linter name in uppercase
docker run \
  -e TEST_CASE_RUN=true \
  -e OUTPUT_DETAIL=detailed \
  -e TEST_KEYWORDS="${name^^}" \
  -e MEGALINTER_VOLUME_ROOT="." \
  -v "/var/run/docker.sock:/var/run/docker.sock:rw" \
  -v "$(pwd)":/tmp/lint \
  "${image_name}"

@ThomasSanson
Copy link
Contributor Author

ThomasSanson commented Mar 1, 2023

note :

if you don't want to mess with other kubernetes linters, just put your test files in a kubernetes_helm folder, and add test_folder: kubernetes_helm in the helm linter descriptor

@nvuillam
Copy link
Member

nvuillam commented Mar 1, 2023

Maybe something more generic like "If your linter requires different test files than in the descriptor default test folder, you can override the test folder by defining test_folder property at linter level , create the folder in .automation/tests and put your test files in it" :)

@ThomasSanson
Copy link
Contributor Author

ThomasSanson commented Mar 2, 2023

run_linter_locally.sh

#!/bin/bash

# Check that the user provided a linter name, ex : kubernetes_helm
if [ -z "$1" ]; then
  echo "$(tput setaf 1) Error: Please provide the name of the linter to run as the first argument like kubernetes_helm $(tput sgr0)"
  exit 1
fi

# Specify the name of the linter to run in lowercase {,,}
name="${1,,}"

# Specify the name of the Docker image to build
image_name="oxsecurity/megalinter-only-${name}"

# Build and tag the Docker image
docker buildx build -f "linters/${name}/Dockerfile" . --tag "${image_name}" || {
  echo "$(tput setaf 1) Error: Failed to build the Docker image. $(tput sgr0)"
  exit 1
}

# Use the case substitution to get the linter name in uppercase
docker run \
  -e TEST_CASE_RUN=true \
  -e OUTPUT_DETAIL=detailed \
  -e TEST_KEYWORDS="${name^^}" \
  -e MEGALINTER_VOLUME_ROOT="." \
  -v "/var/run/docker.sock:/var/run/docker.sock:rw" \
  -v "$(pwd)":/tmp/lint \
  -v "$(pwd)/reports:/tmp/reports" \
  "${image_name}" ||
  {
    echo "$(tput setaf 1) Error: Failed to run the linter. $(tput sgr0)"
    exit 1
  }

@ThomasSanson
Copy link
Contributor Author

ThomasSanson commented Mar 2, 2023

  • Add linter desciptor
  # HELM LINT
  - linter_name: helm
    name: KUBERNETES_HELM
    linter_repo: https://github.com/helm/helm
    linter_url: https://helm.sh/docs/helm/helm_lint/
    linter_version_cache: "v3.11.1"
    files_sub_directory: "" 
    test_folder: kubernetes_helm
    cli_lint_mode: project
    cli_help_arg_name: -h
    cli_lint_extra_args:
      - "lint"
    linter_banner_image_url: https://user-images.githubusercontent.com/19731161/142411871-f695e40c-bfa8-43ca-97c0-94c256749732.png
    linter_text: |
      `helm lint` examine a chart for possible issues.
    examples:
      - helm lint .
      - helm lint --with-subcharts .
    install:
      apk:
        - helm
  • add tests

    • create folder kubernetes_helm
      • create subfolder good
        • add your good file
      • create subfolder bad
        • add your bad file
  • run ./build.sh to generate code

  • run ./run_linter_locally.sh {name in lowercase}

@ThomasSanson
Copy link
Contributor Author

ThomasSanson commented Mar 3, 2023

rm -rf ./site
run npx mega-linter-runner --release beta --flavor python before pushing pull request

@ThomasSanson
Copy link
Contributor Author

@nvuillam Hello,

When I run the command npx mega-linter-runner --release beta --flavor python, it hangs indefinitely and does not return control:

✅ Linted [SPELL] files with [cspell] successfully - (67.86s)
- Using [cspell v6.27.0] https://megalinter.io/cd00bc2af728511f450c232490fa45b5d5417d38/descriptors/spell_cspell
- MegaLinter key: [SPELL_CSPELL]
- Rules config: [/.github/linters/.cspell.json]
- Number of files analyzed: [747]

How can I fix this issue? Is the command I used correct to test at the very end?

@Kurt-von-Laven
Copy link
Collaborator

Kurt-von-Laven commented Mar 4, 2023

That looks right to me; I suspect you're running into #2348, which is the highest priority bug at the moment. Sorry about this. You can probably test on rootful Docker and/or in Windows.

@ThomasSanson
Copy link
Contributor Author

Need to automate venv creation

python3 -m venv .venv
. .venv/bin/activate
echo ".venv/" >> .git/info/exclude
python3 -m pip install -U pip
python3 -m pip install -r requirements.dev.txt

@bdovaz
Copy link
Collaborator

bdovaz commented Mar 8, 2023

Document /build command: #2445 (comment) #2445 (comment)

@echoix
Copy link
Collaborator

echoix commented Mar 8, 2023

Document /build command: #2445 (comment) #2445 (comment)

But only internal collaborators/maintainers can trigger those, we chose to require the write permission for triggering these commands.

@bdovaz
Copy link
Collaborator

bdovaz commented Mar 8, 2023

Document /build command: #2445 (comment) #2445 (comment)

But only internal collaborators/maintainers can trigger those, we chose to require the write permission for triggering these commands.

But it is also clearly indicated on the page (write permission or not), we need to add it in the appropriate section, isn't it?

https://megalinter.io/latest/contributing/#with-write-access

Edit: okay, I just realized it's already documented 😅

@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2023

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Apr 8, 2023
@nvuillam
Copy link
Member

So.... do we have an incoming PR for this issue ? :)

@nvuillam nvuillam removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Apr 15, 2023
@ThomasSanson
Copy link
Contributor Author

So.... do we have an incoming PR for this issue ? :)

Hi there! I apologize for any confusion, as I was actually working on another project where I plan to integrate Mega-Linter (https://github.com/vitabaks/postgresql_cluster).

Rest assured, I'll be addressing this issue and submitting a PR within a maximum of two weeks.

Thanks for your patience and understanding! 😊

@nvuillam
Copy link
Member

@ThomasSanson no need to apologize, everyone works on open-source when available time allows that , there is no deadline, I just wanted to know if I needed to keep this issue open ;)

@ThomasSanson
Copy link
Contributor Author

@nvuillam Thanks for understanding! I'll work on it when I can and keep you updated. You can keep the issue open until I submit the PR. Cheers!

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label May 25, 2023
@nvuillam nvuillam removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label May 26, 2023
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jun 26, 2023
@ThomasSanson
Copy link
Contributor Author

I need to complete this pull request (#2737) before I can address this issue.

@bdovaz bdovaz removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jun 27, 2023
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jul 28, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity
Projects
None yet
Development

No branches or pull requests

5 participants