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

Configuring non root sudoer for molecule tests #96

Merged
merged 3 commits into from
Dec 3, 2024
Merged

Conversation

SequeI
Copy link
Contributor

@SequeI SequeI commented Nov 6, 2024

Adding Redis config which I forgot in initial scenario PR for user_provided
Adding non-root sudoer configuration for our scenario tests to emulate a real user environment
Changed shell commands to ansible collections, such as lineinfile

(Waiting for tf access so I can personally snoop around our molecule scenarios on a tf vm before merge)

@SequeI SequeI requested a review from a team as a code owner November 6, 2024 11:57
@SequeI SequeI marked this pull request as draft November 6, 2024 11:57
@SequeI SequeI force-pushed the asiek/moleculeUser branch 3 times, most recently from a1d33d1 to 91f814f Compare November 11, 2024 04:17
@SequeI SequeI marked this pull request as ready for review November 12, 2024 02:02
Copy link
Collaborator

@bkabrda bkabrda left a comment

Choose a reason for hiding this comment

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

Hi, thanks a lot for the PR. It looks good overall, but I left some comments inline to address. Can you PTAL?

molecule/default/vars/vars.yml Outdated Show resolved Hide resolved
molecule/default/prepare.yml Outdated Show resolved Hide resolved
molecule/default/prepare.yml Outdated Show resolved Hide resolved
molecule/user_provided/prepare.yml Outdated Show resolved Hide resolved
@SequeI SequeI force-pushed the asiek/moleculeUser branch 16 times, most recently from 29448be to d5baf9f Compare November 18, 2024 06:20
Copy link
Collaborator

@bkabrda bkabrda left a comment

Choose a reason for hiding this comment

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

Hi, this looks really good, thanks for all the fixes. I noticed one more place where we could improve, I'll approve this PR once that is done. Thanks!

molecule/testing_user_setup.yaml Outdated Show resolved Hide resolved
@bkabrda
Copy link
Collaborator

bkabrda commented Nov 21, 2024

It seems that something is wrong with the way password is provided/used. From the failed task:

  fatal: [3.142.246.43]: FAILED! => {"msg": "Incorrect sudo password"}

@SequeI SequeI force-pushed the asiek/moleculeUser branch 5 times, most recently from dbad4da to 19f6222 Compare November 24, 2024 22:06
@SequeI
Copy link
Contributor Author

SequeI commented Nov 26, 2024

OIDC instance down so CI won't pass + createtree container fails to spin up on centOS but is fine on RHEL. Will test again once identity instance is back to remove an outside factor in failure but all works as it should on a RHEL machine.

create_home: yes

- name: Set password for testingUser
ansible.builtin.shell: echo {{ item.password }} | passwd --stdin {{ item.user }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Manually setting the password, using user.password previously resulted in constant password errors for some reason

@SequeI
Copy link
Contributor Author

SequeI commented Dec 2, 2024

I constantly ran into the createtree issue before today, but after rebasing with the new Dex OIDC config + changing a small discrepancy it seems to work fine? Still not sure what the underlying issue was

@SequeI SequeI requested a review from bkabrda December 2, 2024 18:00
Copy link
Collaborator

@bkabrda bkabrda left a comment

Choose a reason for hiding this comment

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

Nice job, LGTM. Thanks!

@SequeI SequeI merged commit 7bdc829 into main Dec 3, 2024
6 checks passed
@SequeI SequeI deleted the asiek/moleculeUser branch December 3, 2024 12:37
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.

2 participants