PR#545: add whitelist to fasjson script - fedora-infra/ansible

fedora-infra / ansible

#545 add whitelist to fasjson script

Closed 2 years ago by kevin. Opened 3 years ago by scoady.

fedora-infra/ scoady/ansible add-whitelist into main

add exclude list to fasjson script

Stephen Coady • 2 years ago

140a4fe

roles/fasjson/templates/fasjson-aliases.j2

file modified

+36 -7

		`@@ -10,6 +10,29 @@`
		`aliases_static_file = "/etc/aliases.static"`
		`aliases_file = "/etc/aliases"`

		`+ # we need a list of users/groups`
		`+ # which we dont touch as they are part`
		`+ # of the base template.`
		`+ excluded_entities = [`
		`+ "fonts-sig",`
		`+ "gecko-maint",`
		`+ "hams-sig",`
		`+ "haskell-sig",`
		`+ "i18n-team",`
		`+ "java-sig",`
		`+ "kernel-maint",`
		`+ "lvm-team",`
		`+ "mingwmaint",`
		`+ "mono-sig",`
		`+ "ocamlmaint",`
		`+ "orphan",`
		`+ "packaging-team",`
		`+ "perl-sig",`
		`+ "upstream-release-monitoring",`
		`+ "virtmaint",`
		`+ "xen-maint",`
		`+ "xgl-maint"`
		`+ ]`

		`def gen_all_aliases():`

		`@@ -33,7 +56,8 @@`
		`userinfo = client.get_user(username=username).result`
		`email = userinfo['emails'][0]`

		`- temp.write(f'{username}: {email} \n')`
		`+ if username not in excluded_entities:`
		`+ temp.write(f'{username}: {email} \n')`

		`for group in groups:`
		`groupname = group['groupname']`
		`@@ -44,10 +68,12 @@`
		`groupname=groupname).result`
		`)`
		`if sponsor_list:`
		`- temp.write(`
		`- f'{groupname}-administrators: {groupname}-sponsors \n'`
		`- )`
		`- temp.write(f"{groupname}-sponsors: {sponsor_list} \n")`
		`+ if f'{groupname}-administrators not in excluded_entities:`
		`+ temp.write(`
		`+ f'{groupname}-administrators: {groupname}-sponsors \n'`
		`+ )`
		`+ if f'{groupname}-sponsors not in excluded_entities:`
		`+ temp.write(f"{groupname}-sponsors: {sponsor_list} \n")`

		`member_list = ','.join(`
		`member['username']`
		`@@ -56,7 +82,8 @@`
		`)`

		`if member_list:`
		`- temp.write(f"{groupname}-members: {member_list} \n")`
		`+ if f'{groupname}-members not in excluded_entities:`
		`+ temp.write(f"{groupname}-members: {member_list} \n")`

		`rename(temporary_file.name, aliases_file)`
		`except errors.APIError as e:`
		`@@ -69,7 +96,9 @@`
		`print(f"Something went wrong creating the fasjson client: {e}", file=sys.stderr)`
		`raise`

		`-`
		`+ # this will need to be tested further before it can ever be used`
		`+ # in fact, it probably shouldn't be used as it could change the`
		`+ # entries from aliases.static which is bad.`
		`def update_user(username):`
		`try:`
		`client = Client(url=fasjson_url)`

scoady commented 3 years ago

Signed-off-by: Stephen Coady scoady@redhat.com

Metadata Update from @scoady:
- Pull-request tagged with: freeze-break-request

3 years ago

scoady commented 3 years ago

@kevin this is the solution I've come up with for the fasjson aliases script. The template file with static addresses (and some /dev/null reroutes) needs to stay so I think the best thing to do is whitelist those as users that will never be added by this script.

Let me know what you think.

zuul commented 3 years ago

Build succeeded.

fi-ansible--ansible-review-diff : SUCCESS in 2m 34s

kevin commented 3 years ago

Sounds like a good approach to me. +1

rebased onto 86e76cd2b5ad24c1b34f0c8d2e0e61f43a156ab6

3 years ago

rebased onto 86e76cd2b5ad24c1b34f0c8d2e0e61f43a156ab6

3 years ago

zuul commented 3 years ago

Build succeeded.

fi-ansible--ansible-review-diff : SUCCESS in 3m 50s

mobrien commented 3 years ago

Looks good to me +1

smooge commented 3 years ago

reviewed +1

churchyard commented 3 years ago

Suggestion: "allowlist", or in this case maybe even "excluded_accounts" / "excluded_groups"

Edited 3 years ago by churchyard

kevin commented 3 years ago

+1, agree

rebased onto 6b26ad42c7eb465465403b2bb38dba2b42a2b353

2 years ago

zuul commented 2 years ago

Build succeeded.

fi-ansible--ansible-review-diff : SUCCESS in 2m 05s

churchyard commented 2 years ago

Thanks.

kevin commented 2 years ago

So, @pingou made some other changes to that script... can you rebase this against that?

rebased onto 140a4fe

2 years ago

zuul commented 2 years ago

Build succeeded.

fi-ansible--ansible-review-diff : SUCCESS in 2m 16s

kevin commented 2 years ago

@scoady So, after pingou's changes output is:

postalias: warning: /etc/aliases.db: duplicate entry: "orphan"
postalias: warning: /etc/aliases.db: duplicate entry: "upstream-release-monitoring"
postalias: warning: /etc/aliases.db: duplicate entry: "virtmaint"

scoady commented 2 years ago

@kevin is it possible to tell if those are groups or users? I don't see how it can be adding any of those twice as long as they are users because I've already listed those in excluded_entities. If it's a group it should be orphan-administrators or orphan-members etc.