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

Remove CI IAM user tasks #20

Merged
merged 3 commits into from
Mar 12, 2021
Merged

Remove CI IAM user tasks #20

merged 3 commits into from
Mar 12, 2021

Conversation

vkurup
Copy link
Contributor

@vkurup vkurup commented Feb 2, 2021

This is the accompanying PR to caktus/ansible-role-django-k8s#37

It moves CI user creation to the django-k8s role since it is something that is project-specific (and maybe environment-specific depending on how each project's environments are provisioned). It is now analogous to the S3 bucket creation tasks which are already in that role.

I don't think that this will have any effect on existing projects when they upgrade to this role. This task only changes the deployment the first time that it is run, and then after that it runs each time, but never changes anything. So existing projects wouldn't notice unless they wanted to add another CI IAM user. In that case, they would be able to use the django-k8s role and it would be a very similar experience since the variables are named the same.

@copelco copelco changed the base branch from master to main February 18, 2021 13:50
@vkurup vkurup requested a review from copelco February 18, 2021 14:02
Copy link
Member

@copelco copelco left a comment

Choose a reason for hiding this comment

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

Looks good to me. Added one comment about the README. Maybe add a reference to caktus.django-k8s in the changelog?

@@ -133,44 +133,3 @@ ansible-playbook -l <host/group> echotest.yaml -vv
```sh
ansible-playbook -l <host/group> echotest.yaml --extra-vars "k8s_echotest_state=absent" -vv
```

Copy link
Member

Choose a reason for hiding this comment

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

I think there's also a bullet in the intro of the README that should get removed too

@copelco copelco merged commit 51e8cca into main Mar 12, 2021
@copelco copelco deleted the remove-iam-role-creation branch March 12, 2021 14:03
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