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

Consolidate walle_env_vars #12

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

neoformit
Copy link
Collaborator

Resolves #9

tasks/main.yml Outdated
ansible.builtin.lineinfile:
path: "{{ walle_bashrc }}"
regexp: "^export {{ item.key }}="
line: 'export {{ item.key }}="{{ item.value }}"'
with_items: "{{ walle_env_vars }}"

- name: Add env variables for user deletion (WallE)
- name: Extend walle_bashrc with user-defined walle_extra_env_vars
Copy link
Contributor

Choose a reason for hiding this comment

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

The env vars in the list are merged right? Because if walle_extra_env_vars does not overwrite the walle_env_vars, I would think, these two tasks might overwrite each other on every run, which works, but is not ideal, because it would show changed in the ansible output

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes good point, not ideal! I just changed walle_env_vars to a dict so they can be overridden easily before templating. I think that's a cleaner way to do it?

Copy link
Contributor

@mira-miracoli mira-miracoli left a comment

Choose a reason for hiding this comment

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

Thank you!

with_items: "{{ walle_env_vars }}"
- name: Extend walle_env_vars with user-defined walle_extra_env_vars
ansible.builtin.set_fact:
walle_env_vars: "{{ walle_env_vars | combine(walle_extra_env_vars) }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

cool solution 🚀


# delete users when malware was found and malware severity reached walle_delete_threshold
walle_api_key: null # admin api key to delete users, goes to VAULT
Copy link
Contributor

Choose a reason for hiding this comment

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

They're not appended to the bashrc when null, because they are removed by the combine filter, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question - they are not removed but rendered in walle_bashrc like:

export GALAXY_API_KEY=""
export GALAXY_BASE_URL=""

I think the result in walle.py is the same as if they were unset:

        try:
            if len(from_env := os.environ.get(env, "").strip()) == 0:
                raise ValueError
            else:
                return from_env
        except ValueError:
            logger.error(f"Path for %s is invalid", env)
            raise ValueError

Are you happy with that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, sorry for being over-accurate

Copy link
Collaborator Author

@neoformit neoformit Nov 6, 2024

Choose a reason for hiding this comment

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

Not at all - I appreciate your eyes! 😄

@neoformit neoformit merged commit 5771851 into usegalaxy-eu:main Nov 6, 2024
2 checks passed
@neoformit
Copy link
Collaborator Author

@mira-miracoli To get this installable with ansible-galaxy do we just make another release? 0.0.3?

@mira-miracoli
Copy link
Contributor

I would like to stick to semantic versioning and if everything works, maybe we could release 1.0.0?

@neoformit
Copy link
Collaborator Author

Sounds good to me - I haven't tested your --delete-user functionality but everything else seems ok on our side.
Let me know if there's something specific I can do to help with testing.

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.

Consolidate env variables
2 participants