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

Automated Creating/Updating google groups #248

Conversation

@dims
Copy link
Member

commented May 2, 2019

New collaborators:
        6103BF59 Davanum Srinivas <dims@apache.org>
        D39F838B Christoph Blecker <admin@toph.ca>

`go run reconcile.go`
`go run reconcile.go --dry-run` (to test it)

Sync's the existing google groups that i am aware of, please see:
groups.yaml

Change-Id: I79cbfae654276d38f1c87d7474cc2ba1740340cc

@k8s-ci-robot k8s-ci-robot requested review from mikedanese and spiffxp May 2, 2019

@dims dims force-pushed the dims:add-code-for-maintaining-groups-and-members branch from 95a62e7 to 12262a5 May 2, 2019

@dims dims force-pushed the dims:add-code-for-maintaining-groups-and-members branch 4 times, most recently from 61d0ea9 to 1e788c2 May 3, 2019

@k8s-ci-robot k8s-ci-robot added size/XL and removed size/L labels May 3, 2019

@dims dims changed the title [WIP] Initial cut for Create/Updating google groups Initial cut for Create/Updating google groups May 3, 2019

Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated

@dims dims force-pushed the dims:add-code-for-maintaining-groups-and-members branch 2 times, most recently from 0879bd0 to eb2bb11 May 4, 2019

@dims

This comment has been minimized.

Copy link
Member Author

commented May 4, 2019

/assign @listx

@dims

This comment has been minimized.

Copy link
Member Author

commented May 4, 2019

Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved .git-crypt/.gitattributes
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated

@dims dims force-pushed the dims:add-code-for-maintaining-groups-and-members branch 3 times, most recently from 7c79722 to aaf4462 May 5, 2019

@thockin
Copy link
Member

left a comment

I did not review the Go code :)

This looks great - when the groups are live I will change all the IAM to them.

Show resolved Hide resolved groups/groups.yaml Outdated
Show resolved Hide resolved groups/groups.yaml Outdated
Show resolved Hide resolved groups/groups.yaml Outdated
Show resolved Hide resolved groups/groups.yaml Outdated
@mikedanese
Copy link
Member

left a comment

Go code looks good. Added some final nits.

Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go Outdated

@dims dims force-pushed the dims:add-code-for-maintaining-groups-and-members branch from aaf4462 to fd1821b May 6, 2019

@dims

This comment has been minimized.

Copy link
Member Author

commented May 6, 2019

@mikedanese I think i captured all the points made :) thanks for the very detailed review.

@thockin @spiffxp i think i made all the changes for the mailing list name changes.

@dims

This comment has been minimized.

Copy link
Member Author

commented May 6, 2019

@thockin yes, all the changes are reflected already in the google groups.

@cblecker

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Sorry for the delay, planning to look this one over today.

@spiffxp
Copy link
Member

left a comment

One nit on the dry run flag, I'm otherwise fine with this as is. It would be a good idea to add a README.md to describe intent and and OWNERS file to avoid hitting root

Show resolved Hide resolved groups/reconcile.go
Show resolved Hide resolved groups/reconcile.go Outdated
Show resolved Hide resolved groups/reconcile.go
Show resolved Hide resolved groups/reconcile.go
Show resolved Hide resolved groups/reconcile.go
@cblecker

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Would also like a README and Makefile, but I'm completely okay with that being done in follow up (I can even do Makefile).

@thockin
Copy link
Member

left a comment

What happened to the k8s-infra-team-private+letsencrypt@googlegroups.com entry? That is used by letsencrypt via cert-manager

Show resolved Hide resolved groups/groups.yaml
@thockin
Copy link
Member

left a comment

When this merges I can start the transition

@dims dims force-pushed the dims:add-code-for-maintaining-groups-and-members branch from fd1821b to 1764856 May 9, 2019

@dims

This comment has been minimized.

Copy link
Member Author

commented May 9, 2019

I don't think i am in the k8s-infra-team-private+letsencrypt@googlegroups.com, please let me know the list of members of that ML @thockin and i can add them to groups.yaml (or we can do that in a follow up)

Initial cut for Create/Updating google groups
New collaborators:
	6103BF59 Davanum Srinivas <dims@apache.org>
	D39F838B Christoph Blecker <admin@toph.ca>

`go run reconcile.go`
`go run reconcile.go --dry-run` (to test it)

Sync's the existing google groups that i am aware of, please see:
groups.yaml

Change-Id: I33d0eccb3f9087d339323ca35a425dff2416b9d4

@dims dims force-pushed the dims:add-code-for-maintaining-groups-and-members branch from 1764856 to 3d6367d May 9, 2019

@spiffxp

This comment has been minimized.

Copy link
Member

commented May 9, 2019

/hold
would like to see @thockin and @cblecker weigh in then this can be removed

/approve
I'm happy with where this is at

EDIT: changed my mind, christoph asked that we not block on him, I think tim's concerns have been address

I leave it to @dims to remove the /hold when ready

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented May 9, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: spiffxp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@spiffxp

This comment has been minimized.

Copy link
Member

commented May 9, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label May 9, 2019

@thockin thockin changed the title Initial cut for Create/Updating google groups Automated Creating/Updating google groups May 9, 2019

@thockin
Copy link
Member

left a comment

groups.yaml LGTM

When this merges and is fully actuated, let me know and I can flip all the IAM (tedious -- let's do it ONE TIME :)

# Configuration for the kubernetes.io Google Groups setup

# file path of the google groups service account token
token-file: k8s-infra-test-project-1896690daeb3.json

This comment has been minimized.

Copy link
@thockin

thockin May 9, 2019

Member

The fact this says "test" suggests we should move it

This comment has been minimized.

Copy link
@dims

dims May 9, 2019

Author Member

We can rename this when we have the new service account @thockin

token-file: k8s-infra-test-project-1896690daeb3.json

# Email id of the bot service account
bot-id: wg-k8s-infra-api-test@kubernetes.io

This comment has been minimized.

Copy link
@thockin

thockin May 9, 2019

Member

I thought @spiffxp wants to remove the account - do wee need it?

This comment has been minimized.

Copy link
@dims

dims May 9, 2019

Author Member

yes we need it. we use this in the code around line 110

credential.Subject = config.BotID

and the subject field is

// Subject is the optional user to impersonate.
@dims

This comment has been minimized.

Copy link
Member Author

commented May 9, 2019

/hold cancel

@k8s-ci-robot k8s-ci-robot merged commit 12cafbd into kubernetes:master May 9, 2019

2 of 3 checks passed

tide Not mergeable. Should not have do-not-merge/hold label.
Details
cla/linuxfoundation dims authorized
Details
pull-k8sio-cip Skipped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.