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

feat: individual support for manual checks and exceptions #84

Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
808f9e8
chore(deps): bump splunk-appinspect from 2.14.1 to 2.30.0
dependabot[bot] Dec 7, 2022
ca2f209
test: dockerfile instead of image
truptilangalia-crest Dec 15, 2022
67deec7
test: failed action on not vetting entry
truptilangalia-crest Dec 15, 2022
4afd00e
test: resolved issue with failures
truptilangalia-crest Dec 15, 2022
718b7f5
test: updated conditions for checking vetting file
truptilangalia-crest Dec 16, 2022
49c7d36
test: fixed an error
truptilangalia-crest Dec 16, 2022
b2ae886
test: code added for 2 vetting files
truptilangalia-crest Dec 16, 2022
1c00fdb
test: fixed typo in condition
truptilangalia-crest Dec 19, 2022
439e099
test: markdown for expected failures
truptilangalia-crest Dec 19, 2022
376a46c
test: file open with append mode
truptilangalia-crest Dec 19, 2022
b2966f4
test: testing
truptilangalia-crest Dec 19, 2022
6d8c397
test: reverted changes
truptilangalia-crest Dec 19, 2022
4bc7bd1
test: removed manual tag condition for generating markdown
truptilangalia-crest Dec 19, 2022
954be01
test: formated result
truptilangalia-crest Dec 20, 2022
672d040
test: formatted results
truptilangalia-crest Dec 20, 2022
2cb9f9f
tets: format changed for failures
truptilangalia-crest Dec 20, 2022
b33d646
test: fixed a typo
truptilangalia-crest Dec 20, 2022
1c050e5
test: warjings added
truptilangalia-crest Dec 20, 2022
96f0f59
test: validation added for comment
truptilangalia-crest Dec 21, 2022
0406b25
test: re imported
truptilangalia-crest Dec 21, 2022
2196b6a
test: fixed a tyopo
truptilangalia-crest Dec 21, 2022
39926c2
test: comments added
truptilangalia-crest Dec 21, 2022
50b70ed
test: added checklist
truptilangalia-crest Dec 21, 2022
15724d7
test: added list
truptilangalia-crest Dec 21, 2022
3ab75c8
test: typo fixed
truptilangalia-crest Dec 21, 2022
bbc3465
test: updated file names
truptilangalia-crest Dec 23, 2022
2231123
test: added review comment
truptilangalia-crest Dec 26, 2022
2ce3dad
test: format related changes
truptilangalia-crest Dec 27, 2022
44a2d25
test: removed exttra debug statements
truptilangalia-crest Dec 28, 2022
8de57d2
test: replaced set-output
truptilangalia-crest Dec 28, 2022
81a9995
test: code cleanup
truptilangalia-crest Jan 2, 2023
ce04283
test: file name changed
truptilangalia-crest Jan 2, 2023
bc68710
test: variable name changes
truptilangalia-crest Jan 3, 2023
d2679e8
test: readme changed
truptilangalia-crest Jan 3, 2023
b07851f
test: resolved review comments
truptilangalia-crest Jan 3, 2023
5f51660
test: pre-commit fixes and added yaml extension
truptilangalia-crest Jan 4, 2023
be7541a
test: conflict from main
truptilangalia-crest Jan 4, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile
Expand Up @@ -12,4 +12,4 @@ COPY export_to_markdown.py /

# Code file to execute when the docker container starts up (`entrypoint.sh`)
WORKDIR /github/workspace
ENTRYPOINT ["bash", "-x", "/entrypoint.sh"]
ENTRYPOINT ["bash", "/entrypoint.sh"]
57 changes: 43 additions & 14 deletions README.md
@@ -1,7 +1,7 @@
# Splunk AppInspect action

This action runs Splunk's AppInspect CLI against a provided a directory of a Splunk App.
It fails if the result contains any failures.
This action runs Splunk's AppInspect CLI against a provided directory of Splunk App.
It fails if the result contains any failures or manual checks are not vetted.

The (json) result will be written to the file specified with [`result-file`](#result-file).
This can be uploaded for later viewing to use in another step/job using [`actions/upload-artifact@v2`](https://github.com/marketplace/actions/upload-a-build-artifact).
Expand Down Expand Up @@ -29,35 +29,58 @@ Appinspect tags to exclude

`required`: `false`

### `app_vetting`
Path to app vetting yaml file. Used only if `manual` in `included_tags`
### `appinspect_manual_checks`
Path to file which contains list of manual checks

`default`: `.app-vetting.yaml`
`required`: `false`
`default`: `.appinspect.manualcheck`

### `appinspect_expected_failures`
Path to file which contains list of expected appinspect failures

`required`: `false`
`default`: `.appinspect.expect`

### `manual_check_markdown`
Path for generated file with markdown for manual checks. Used only if `manual` in `included_tags`
Path to generated file with markdown for manual checks

`required`: `false`
`default`: `manual_check_markdown.txt`

### `appinspect_expected_failures`
Path to generated file with markdown for expected appinspect failures

`required`: `false`
`default`: `expected_failure_markdown.txt`

## Outputs

### `status`:

`pass|fail`

## Using manual tag
Running `appinspect-cli-action` with `manual` tag in `included_tags` detects checks that need to be verified manually and tests if all of them were already reviewed - if not the action will fail.
### Manual checks review
To see checks to be verified inspect the `result_file` from `appinspect-cli-action` run with manual tag. Verify manual checks and mark them as reviewed by adding them one by one into `.app-vetting.yaml`, ex:
To see checks to be verified, inspect the `result_file` from `appinspect-cli-action`. Verify manual checks and mark them as reviewed by adding them one by one into `.appinspect.manualcheck`, ex:
```yml
name_of_manual_check_1:
comment: 'your comment'
name_of_manual_check_2:
comment: 'your comment'
```
please note that names of validated manual checks should be aligned with those from `result_file` and your comment can't be empty.
Please note that names of validated manual checks should be aligned with those from `result_file` and your comment can't be empty.

### Failure checks review
To mark Failures as expected, add them into `.appinspect.expect` with proper comment containing ticket id of ADDON/APPCERT project associated with the exception, ex:
```yml
name_of_exception_1:
comment: 'ADDON-123: your comment'
name_of_exception_2:
comment: 'APPCERT-123: your comment'
```
Please note that your comment can't be empty, it must include ticket id of ADDON/APPCERT project associated with the exception and the names of exceptions should be aligned with those from `result_file`.

### Running the job
When `appinspect-cli-action` is called with `manual` tag, it scans the package with Splunk's AppInspect CLI and searches for manual checks. In the next step, action compares `results_file` with `.app-vetting.yaml` if any check wasn't reviewed and isn't in `.app-vetting.yaml` then the job fails.
When `appinspect-cli-action` is called, it scans the package with Splunk's AppInspect CLI. If there are any failures observed then action compares `results_file` with `.appinspect.expect`. If that failure isn't present in `appinspect.expect` or it does not contain an appropriate comment(containing ADDON/APPCERT ticket id associated with the exception) then the job fails with proper failure reason. In the next step, action compares `results_file` with `.appinspect.manualcheck`. If any manual check wasn't reviewed by addon developer and isn't in `.appinspect.manualcheck` then the job fails.

## Example usage

Expand All @@ -66,20 +89,26 @@ When `appinspect-cli-action` is called with `manual` tag, it scans the package w
with:
app_path: 'test'
```
### Downloading manual checks markdown
If the comparison is successful then a markdown consisting a table with manual check names and comments is generated. It can be uploaded to artifacts.
### Downloading markdowns
If the comparison is successful then a markdown consisting a table with check names and comments is generated. It can be uploaded to artifacts.
```yml
- uses: actions/checkout@v2
- uses: splunk/appinspect-cli-action@v1.3
with:
app_path: 'test'
included_tags: manual
included_tags: {appinspect-tags-to-include}
manual_check_markdown: manual_check_markdown.txt
expected_failure_markdown: expected_failure_markdown.txt
- name: upload-manual-check-markodown
uses: actions/upload-artifact@v2
with:
name: manual_check_markdown.txt
path: manual_check_markdown.txt
- name: upload-expected_failure-markodown
uses: actions/upload-artifact@v2
with:
name: expected_failure_markdown.txt
path: expected_failure_markdown.txt
```
The markdown is ready to paste into confluence, by:
`Edit -> Insert more content -> Markup`, change insert type to `Markdown` and paste the contents of the file.
22 changes: 15 additions & 7 deletions action.yml
@@ -1,9 +1,9 @@
# action.yml
name: "Splunk AppInspect"
description: "Run Splunk App insect on a Splunk app directory."
description: "Run Splunk App inspect on a Splunk app directory."
inputs:
app_path:
description: "path to the application directory to be inspected"
description: "Path to the application directory to be inspected"
default: build/splunkbase
result_file:
description: "json result file name"
Expand All @@ -14,17 +14,25 @@ inputs:
excluded_tags:
description: "Tags to exclude"
required: false
app_vetting:
description: "Path to app vetting yaml file"
appinspect_manual_checks:
description: "Path to file which contains list of manual checks"
required: false
default: ".app-vetting.yaml"
default: ".appinspect.manualcheck"
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you think about keeping .yaml extension in the file name? it's more clear what kind of data is inside

appinspect_expected_failures:
description: "Path to file which contains list of expected appinspect failures"
required: false
default: ".appinspect.expect"
manual_check_markdown:
description: "Path for generated file with markdown for manual checks and exceptions"
description: "Path for generated file with markdown for manual checks"
required: false
default: "manual_check_markdown.txt"
expected_failure_markdown:
description: "Path for generated file with markdown for expected appinspect failures"
required: false
default: "expected_failure_markdown.txt"
outputs:
status:
description: "value is success/fail based on app inspect result"
runs:
using: "docker"
image: "docker://ghcr.io/splunk/appinspect-cli-action/appinspect-cli-action:v1.5.0"
image: "Dockerfile"
35 changes: 27 additions & 8 deletions compare_checks.py
Expand Up @@ -4,6 +4,7 @@
from typing import List

import yaml
import re

print(
f"{os.path.basename(__file__)} script was called with parameters: {' '.join(sys.argv[1:])}"
Expand All @@ -25,19 +26,27 @@ class BCOLORS:
BOLD = "\033[1m"
UNDERLINE = "\033[4m"

def validate_comment(vetting_data):
checks = []
ticket_id=re.compile(r"((?i)(ADDON|APPCERT)-[0-9]+)")
for check, info in vetting_data.items():
if not re.search(ticket_id,info.get("comment")):
checks.append(check)
return checks


def compare(
check_type: str,
vetting_file: str = ".app-vetting.yaml",
vetting_file: str = ".appinspect.manualcheck",
appinspect_result_file: str = "appinspect_output.json",
) -> List[str]:
"""
Compares checks from vetting file and appinspect result file. A lot prints are added to make it
easier for users to create proper vetting_file and understand errors

:param vetting_file: path to yaml file with verified manual checks
:param vetting_file: path to file with varified list of checks
:param appinspect_result_file: path to Splunk's AppInspect CLI result file
:return: list of non matching tests between vetting_file and appinspect_result_file or not commented ones
:return: list of non matching tests between vetting_file and appinspect_result_file or not commented ones or checks with inappropriate comment
"""
if not os.path.isfile(appinspect_result_file):
raise FileNotFoundError(
Expand Down Expand Up @@ -89,28 +98,38 @@ def compare(
print(
f"{BCOLORS.FAIL}{BCOLORS.BOLD}Please see appinspect report for more detailed description about {check_type} checks and review them accordingly.{BCOLORS.ENDC}"
)
checks_with_no_id = []
if check_type=="failure":
checks_with_no_id = validate_comment(vetting_data)
if checks_with_no_id:
print(
f"{BCOLORS.FAIL}{BCOLORS.BOLD}There are some checks which require comment with proper ticket id in {vetting_file}. Below checks are not commented with required ticket id"
f" {vetting_file}:{BCOLORS.ENDC}"
)
for check in checks_with_no_id:
print(f"{BCOLORS.FAIL}{BCOLORS.BOLD}\t{check}{BCOLORS.ENDC}")

return new_checks + not_commented
return new_checks + not_commented + checks_with_no_id


def get_checks_from_appinspect_result(
path: str, result: str = "manual_check"
) -> List[str]:
"""
Returns manual checks from appinspect json result file
Returns checks from appinspect json result file

:param path: path to json result file
:return: list of checks in string format
"""
manual_checks = []
checks = []
with open(path) as f:
appinspect_results = json.load(f)
for report in appinspect_results["reports"]:
for group in report["groups"]:
for check in group["checks"]:
if check["result"] == result:
manual_checks.append(check["name"])
return manual_checks
checks.append(check["name"])
return checks


def main():
Expand Down
26 changes: 14 additions & 12 deletions entrypoint.sh
Expand Up @@ -42,23 +42,25 @@ python3 /reporter.py $INPUT_RESULT_FILE
exit_code=$?
echo "::endgroup::"

exit_code_failure_check=$exit_code
if [ $exit_code != 0 ]; then
echo "::group::failure_checks"
python3 /compare_checks.py $INPUT_APP_VETTING $INPUT_RESULT_FILE "failure"
exit_code=$?
python3 /compare_checks.py $INPUT_APPINSPECT_EXPECTED_FAILURES $INPUT_RESULT_FILE "failure"
exit_code_failure_check=$?
echo "::endgroup::"
fi

if [[ "$INPUT_INCLUDED_TAGS" == *"manual"* ]] && [ $exit_code == 0 ]; then
echo "::group::manual_checks"
python3 /compare_checks.py $INPUT_APP_VETTING $INPUT_RESULT_FILE "manual_check"
exit_code=$?
if [ $exit_code == 0 ]; then
echo "successful comparison, generating markdown"
echo "/export_to_markdown.py $INPUT_APP_VETTING $INPUT_MANUAL_CHECK_MARKDOWN"
python3 /export_to_markdown.py $INPUT_APP_VETTING $INPUT_MANUAL_CHECK_MARKDOWN
fi
echo "::group::manual_checks"
python3 /compare_checks.py $INPUT_APPINSPECT_MANUAL_CHECKS $INPUT_RESULT_FILE "manual_check"
exit_code_manual_check=$?
echo "::endgroup::"

if [ $exit_code_failure_check == 0 ] && [ $exit_code_manual_check == 0 ] ; then
echo "::group::generate_markdown"
echo "successful comparison, generating markdown"
python3 /export_to_markdown.py $INPUT_APPINSPECT_MANUAL_CHECKS $INPUT_MANUAL_CHECK_MARKDOWN
python3 /export_to_markdown.py $INPUT_APPINSPECT_EXPECTED_FAILURES $INPUT_EXPECTED_FAILURE_MARKDOWN
echo "::endgroup::"
fi

exit "$exit_code"
exit "$(($exit_code_failure_check || $exit_code_manual_check))"
Copy link
Contributor

Choose a reason for hiding this comment

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

newline needed - please fix pre-commit and CI

28 changes: 14 additions & 14 deletions export_to_markdown.py
Expand Up @@ -9,13 +9,13 @@
<table class=3D"confluenceTable">
<tbody>
<tr>
<th class=3D"confluenceTh">manual check</th>
<th class=3D"confluenceTh">check</th>
<th class=3D"confluenceTh">comment</th>
</tr>
"""

CHECK_MARKDOWN_TEMPLATE = """<tr>
<td class=3D"confluenceTh">{manual_check}</th>
<td class=3D"confluenceTh">{check}</th>
<td class=3D"confluenceTh">{comment}</th>
</tr>
"""
Expand All @@ -30,36 +30,36 @@ class ExportToMarkdown:
Based on app vetting file generates file with markdown consisting names of validated checks and comments.
"""

def __init__(self, manual_checks_path, markdown_output_path):
self.manual_checks_path = manual_checks_path
def __init__(self, checks_path, markdown_output_path):
self.checks_path = checks_path
self.markdown_output_path = markdown_output_path
self.manual_checks = None
self.checks = None

def __call__(self):
self._load_manual_checks()
self._load_checks()
self._create_output_markup()

def _load_manual_checks(self):
with open(self.manual_checks_path) as vetting_data:
self.manual_checks = yaml.safe_load(vetting_data)
if self.manual_checks is None:
self.manual_checks = {}
def _load_checks(self):
with open(self.checks_path) as vetting_data:
self.checks = yaml.safe_load(vetting_data)
if self.checks is None:
self.checks = {}

def _create_output_markup(self):
with open(self.markdown_output_path, "w") as output:
output.write(MARKDOWN_START)
for manual_check, check_attributes in self.manual_checks.items():
for check, check_attributes in self.checks.items():
output.write(
CHECK_MARKDOWN_TEMPLATE.format(
manual_check=manual_check, comment=check_attributes["comment"]
check=check, comment=check_attributes["comment"]
)
)
output.write(MARKDOWN_END)


def main():
ExportToMarkdown(
manual_checks_path=APP_VETTING_PATH, markdown_output_path=MARKDOWN_OUTPUT_PATH
checks_path=APP_VETTING_PATH, markdown_output_path=MARKDOWN_OUTPUT_PATH
)()


Expand Down