This smell can be detected in two different ways for Puppet:

1. More than one resource is defined in the declaration of a file, service or package. We are not currently able to support this because of #7.
2. Calculation of the LCOM. LCOM is related to the intersection of parameters between components. We should be able to support this easily if we implement #6.

The print should be recursive and follow the same patterns as the other components, otherwise it becomes hard to understand what is going on.

Instead of this:

roles[0]->None attributes: [name:'Install dependencies']

We want something like this:

roles[0]->None:
  attributes:
    name->Install dependencies

Describe the solution you'd like
The CLI should have a command that allows to print the intermediate representation for a certain script.

Describe the solution you'd like
It would be interesting to have support in the intermediate representation for the management of nodes, i.e., for instance the inventory in Ansible and the node construct in Puppet.

Describe the bug
setuptools is required since we use the module pkg_resources

Describe the solution you'd like
Right now, the UnitBlock has an attribute for each type of element. However, this does not scale well, does not adhere to good practices of object-oriented programming and it is not intuitive when generic statements are in the mix. For instance, let's imagine a conditional statement has a atomic unit in its blocks. Should the atomic unit also be added to the atomic_units attribute? It doesn't make sense.

Describe alternatives you've considered
The UnitBlock should have a single attribute for statements.

We ignored the point "Right-to-left chaining arrows should not be used", since we are not able to support this Puppet operation yet.

Describe the solution you'd like
It would be nice to have black in the CI. This would enforce the usage of black.

Currently doing checksums with md5 will trigger weak crypt smell, eg:

# Docker
RUN md5sum foo.sh

In this case md5sum is being used to verify the integrity of the file and it will trigger the weak crypt smell. md5sum and other checksums commands (shasum, sha1sum, etc..) should be whitelisted.

Since autodetect was removed, the extension has to be updated.

Describe the solution you'd like
It would be nice to have an automated test that checks if the number of true/false positives and true/false negatives remains the same for the oracle datasets used in GLITCH's studies.

In Ansible, Chef and Puppet, there are files that exist only to define variables. In those files, the smell should not be triggered.

Describe the bug
Long statement is being detected when we have 140 characters + '\n'.

To Reproduce
Run GLITCH on script with a line with 140 characters + '\n'.

Expected behavior
It shouldn't detect the smell

Is your feature request related to a problem? Please describe.
Currently GLITCH does not support attributes defined as in the example below (aka Ansible-specific syntax):

- name: Create web root
  file: path="{{ www_root }}"
        owner="{{ web_user }}"
        group="{{ web_group }}"
        mode=0755
        state=directory
  with_dict: sites

This is mentioned in the work by Opdebeeck et al. (2023).

Describe the solution you'd like
We shoud use the ansible-core package instead of the yaml package..

Describe the solution you'd like
We should migrate the VSCode extension to the Python tools template (https://github.com/microsoft/vscode-python-tools-extension-template#readme). This would allow GLITCH to be bundled in the extension.

Describe the solution you'd like
Right now some CLI options are not very clear. For instance, the --includeall and --dataset are not very clear and should be replaced with simpler options or even removed. The --linterand --csv option could also be replaced with a format option.

Puppet

• Collect dataset (Rahman)
• Compute and validate dataset statistics
• Collect Oracle (Our own)
• Compute and validate oracle statistics
• Run SLIC: collect results and execution time
  • Oracle
  • Compute precision and accuracy
  • Rahman's dataset (7 smells) 🎯
• Run GLITCH: collect results and execution time
  • Oracle
  • Compute precision and accuracy
  • Rahman's dataset (7 smells) 🎯
  • Rahman's dataset (all of GLITCH's supported smells) 🎯

Ansible

• Collect dataset (our own, following same criteria as Rahman and Rahman Openstack Dataset)
• Compute and validate dataset statistics
• Collect Oracle (Rahman)
• Compute and validate oracle statistics
• Review Oracle and add code line to each smell
• Run SLAC: collect results and execution time
  • Oracle 🎯
  • Compute precision and accuracy 🎯
  • Rahman's dataset (8 smells)
• Run GLITCH: collect results and execution time
  • Oracle 🎯
  • Compute precision and accuracy 🎯
  • Rahman's dataset (8 smells)
  • Rahman's dataset (all of GLITCH's supported smells)

Chef

• Collect dataset (our own, following same criteria as Rahman)
• Compute dataset statistics
• Collect Oracle (Our own)
• Compute and validate oracle statistics
• Run SLAC: collect results and execution time
  • Oracle
  • Compute precision and accuracy
  • Run on dataset (9 smells) 🎯
• Run GLITCH: collect results and execution time
  • Oracle
  • Compute precision and accuracy
  • Run on dataset (9 smells) 🎯
  • Run on dataset (all of GLITCH's supported smells; same as above?) 🎯

It would be interesting if there was a better way to define the type of an Ansible script (vars, tasks or script)

Following the discussion here, we should decide if hashes are handled as hierarchical attributes and variables in Chef and Puppet, or should be handled as regular values.

(#17)

AttributeError: 'str' object has no attribute '__name__'

(or we should adapt the current ones)
Some technologies have blocks of structures that do not make sense in every technology. For instance, Puppet allows to group resources inside the same declaration

https://github.com/Nfsaavedra/GLITCH/blob/5162cd1168e2ccfb28373a51ef0fff9a9423667f/glitch/analysis/rules.py#L279

The parsers for the intermediate representation should have unit tests.

Describe the solution you'd like
Unit tests.

Describe the solution you'd like
Refactor the tests to use pytest instead of unittest.

          This is fine, but it might be a good idea to include versions (perhaps the best is to create a separate issue for that). Otherwise, we might have issues in a near future related to incompatible versions.

Originally posted by @jff in #19 (comment)

Describe the bug
The regex for the missing integrity check isn't triggered on values as such:
https://storage.googleapis.com/cri-containerd-release/cri-containerd-{{ containerd_version }}.linux-amd64.tar.gz
This happens because of the space before and after the variable.

To Reproduce
Run GLITCH on this script:
https://github.com/starlingx/ansible-playbooks/blob/7983841637966089106bb80f28d7b701ec6b6323/playbookconfig/src/playbooks/roles/provision-edgeworker/prepare-edgeworker/kubernetes/tasks/install-ubuntu-packages.yml#L31

Expected behavior
Detecting a Missing integrity check smell.

We should parse the values in the intermediate representation allowing to differ, for instance, types (booleans, strings, numbers...) and expressions (ands, ors ...). This parsing would allow to define other type of smells in a more accurate way. For instance, imagine the smell "Hard-coded secret". We have something like: $test | "hello". Although a variable is present, there is still a chance that the secret is hard-coded.

Is your feature request related to a problem? Please describe.
Right now the condition statement and its conditions are represented with the same construct ConditionStatement. However, this doesn't allow the distinction between them and sometimes the conditions are used as being the condition statement itself.
For instance:

$php_prefix = $::osfamily ? {
    'debian' => 'php5-',
    'redhat' => 'php-',
}

Only has a ConditionStatement for the first condition and one for the second condition, but it doesn't have a construct for the actual switch statement.

Describe the solution you'd like
We should create a new construct either for the conditions or the switch/if statements.

Describe the solution you'd like
Change the usage of the package click to the package fire for the CLI.

Describe the bug
The tests are creating a Dockerfile that is not deleted

Expected behavior
The file is deleted.

We only consider duplicate blocks inside the same file instead of inside the same cookbook (as Schwarz et al.)