ansible / validated-content-discussion

A place for review of validated content candidates and general discussion around validated content.
GNU General Public License v3.0
2 stars 4 forks source link

candidate: infra.ansible_rekey_variables #22

Closed AliAkkaya7 closed 3 months ago

AliAkkaya7 commented 8 months ago

Candidate Name

infra.ansible_rekey_variables

Link to Github Repo

https://github.com/AliAkkaya7/ansible_rekey_variables

Review Due By

CHECKLIST

Business Checks:

Content Viability

Content Duplication

Technical Checks:

Content Conformity

Content Compliance

Content Testing/Validation

alisonlhart commented 7 months ago

@AliAkkaya7 There are a number of items to address before bringing this to the Automation Community of Practice for review. Holding for review until more of these requirements are met.

alisonlhart commented 5 months ago

@AliAkkaya7 Touching base on this! Are there any updates on the remaining checklist items?

AliAkkaya7 commented 5 months ago

@alisonlhart I reviewed the requirements and all checklists that can be achieved are done. For example, OpenSSF Best Practices are mostly applied but still there are some parts like tests etc. that are not in place. I don't have experience in this area and if it is required then maybe I can proceed with your help.

Currently the collection is under my Namespace in Galaxy which is aliakkaya7.ansible_rekey_variables. Because I don't have write access from my repository to build collections under the infrastructure.

AliAkkaya7 commented 5 months ago

@alisonlhart is there any progress? I did all I can do, the remaining is related to locate the collection. Currently it is in my namespace in Ansible Galaxy.

alisonlhart commented 5 months ago

@djdanielsson @arnav3000 @ericzolf Can you take a look at this candidate?

djdanielsson commented 5 months ago

a few things I saw when skimming over it 1) README's say ansible min version is 2.9 not 2.14.0 which is now required and is correct in the collection meta runtime file. (Also meta in the role itself says 2.9) 2) I do not see any Ansible-lint rules and when looking at the last action run it shows it is not using a profile.

ansible-lint 24.2.0 using ansible-core:2.16.4 ansible-compat:4.1.11 ruamel-yaml:0.18.6 ruamel-yaml-clib:0.2.8
Run ansible-lint 
  ansible-lint 
  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}

3) IMO 'rekey_variable_log_var' is not a clear name as to what it does. rekey_no_log_var or rekey_secure_logging I think are more clear for example 4) In the file get_content.yml the first 3 tasks are setting facts which appear to not depend on each other and have the same tags so I think it would be best to combine all 3 into 1 task for efficiency

AliAkkaya7 commented 4 months ago

@djdanielsson thanks for your comment. I fixed them except for the production profile in the pipeline. I added the profile parameter in the .ansible-lint file but it is not getting the production profile in the workflow. Can you let me know how I can configure it?

djdanielsson commented 4 months ago

hum... the action has very little docs, I think you might be able to try and change the version you are running to either @main or @v24 and see if that changes anything. If that doesn't work let me know and I can try a few other things on the side.

AliAkkaya7 commented 4 months ago

@djdanielsson I changed all to the main but still I see "shell: /usr/bin/bash --noprofile --norc -e -o pipefail" in pipeline.

djdanielsson commented 4 months ago

interesting, that must just be how the action works, I double checked the output. it shows "Passed: 0 failure(s), 0 warning(s) on 30 files. Profile 'production' was required, and it passed." which means it is reading the profile that is now being set in the ansible-lint config so it is working correctly.

alisonlhart commented 4 months ago

@AliAkkaya7 Not to distract from the ongoing conversation, but since the ansible-lint action is working correctly now, the last item should be to migrate it into the redhat-cop Github org. You can do this by opening a "Repository Management" issue in the redhat-cop/org repo. Specify that this is a migration of existing repository. That group will help you move the content over. Once this is complete, please re-tag me again and we'll do a final sign-off on the content.

AliAkkaya7 commented 4 months ago

@alisonlhart created https://github.com/redhat-cop/org/issues/769.

alisonlhart commented 4 months ago

Readding this to the ACoP call agenda, for April 17th call.

alisonlhart commented 4 months ago

@AliAkkaya7 Don't forget to add the OpenSSF badge to the repo as well! (RE checklist item: Repo is self-certified with the [OpenSSF Best Practices](https://bestpractices.coreinfrastructure.org/en) and has the OpenSSF Best Practices Badge)

alisonlhart commented 4 months ago

Approved by the ACoP! After the final items are complete (OpenSSF badge and redhat-cop repo migration), this can be released to Automation Hub. @AliAkkaya7 Please reach out to me on slack @allhart to set up access to the infra namespace.

AliAkkaya7 commented 4 months ago

Great News, thanks @alisonlhart! I will be back on 29 April, I will reach out to you when I am back. There is just the test phase missing in OpenSSF badge. I am not sure if it fits with my repo or how to enable it automatically.