Refactor the GSuite Reconciler #3407

upodroid · 2022-02-16T15:10:32Z

Fixes: #3356

There are a few prowjobs that need to be updated.

/cc @dims @evankanderson

k8s-ci-robot · 2022-02-16T15:10:35Z

@upodroid: GitHub didn't allow me to request PR reviews from the following users: evankanderson.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Fixes: #3356

There are a few prowjobs that need to be updated.

/cc @dims @evankanderson

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

linux-foundation-easycla · 2022-02-16T15:10:36Z

The committers are authorized under a signed CLA.

✅ Mahamed (0bce68c)

k8s-ci-robot · 2022-02-16T15:10:40Z

Welcome @upodroid!

It looks like this is your first PR to kubernetes/k8s.io 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/k8s.io has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2022-02-16T15:10:41Z

Hi @upodroid. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dims · 2022-02-16T15:40:15Z

/assign @cblecker

evankanderson

A few cleanup comments -- a couple are musings on the code that should at most result in a TODO rather than any actual code changes.

I'm also suggesting you walk back a few of the code changes, because the validation seems Kubernetes-specific enough that it's not worth trying to make it generic right now (and we can always figure out how to do that in the future if someone ends up wanting something similar).

groups/main.go

evankanderson · 2022-02-17T02:49:25Z

groups/main.go

+	}
+
+	PrintConfig(config)
+	err = restrictionsConfig.Load(config.RestrictionsPath)


It's odd that there is a restrictions-path flag, but it's not used here.

Should config be used to specify these values for the validate command, or should the plan and apply commands consider the restrictions-path flag?

I'm concerned that having two different ways to load this will cause someone to call validate with different settings that will actually be used for apply, and they will think that they are doing something valid and safe when they actually aren't.

evankanderson · 2022-02-17T02:49:51Z

groups/main.go

+		log.Fatal(err)
+	}
+
+	err = groupsConfig.Load(config.GroupsPath, &restrictionsConfig)


Ditto with groups-path.

groups/main.go

groups/validate.go

evankanderson · 2022-02-17T03:20:24Z

groups/validate.go

-	for _, g := range cfg.Groups {
-		if g.EmailId == "gke-security-groups@kubernetes.io" {
+	for _, g := range groupsConfig.Groups {
+		if g.EmailId == "gke-security-groups@"+primaryDomain {


I'm not sure swapping this for primaryDomain helps much in terms of portability. I'd leave this as k8s-specific and just opt people out of this test.

This is a useful for feature for anyone doing Google Groups RBAC for GKE. We'll be using this in Knative. In GSuite, there is a primary domain and secondary domains and key features are tied to the primary domain(item 0 in the list)

https://support.google.com/a/answer/7009324?hl=en

evankanderson · 2022-02-17T03:22:06Z

groups/validate.go

 	}
 	for email, found := range rbacEmails {
 		if !found {
-			t.Errorf("group '%s': must be a member of gke-security-groups@kubernetes.io", email)
+			log.Printf("group '%s': must be a member of gke-security-groups@"+primaryDomain, email)


A 👎 on combining log.Printf and string + -- but I'd switch these back to @kubernetes.io anyway.

groups/validate.go

dims · 2022-03-09T17:44:05Z

/ok-to-test

upodroid · 2022-03-09T17:50:59Z

Forgot about this, will address the remaining feedback from Evan by tomorrow.

puerco · 2022-03-10T00:27:28Z

CI failure fixed in #3495
/test pull-k8sio-verify

…nciler

upodroid · 2022-03-10T17:14:56Z

~~@puerco The CI is still broken. The yamllint error is for k8s.gcr.io/* files. Did rebase from main now.~~

nvm main.go has bad headers

k8s-triage-robot · 2022-06-08T18:19:02Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

evankanderson · 2022-06-14T20:47:46Z

/remove-lifecycle-stale

k8s-triage-robot · 2022-07-14T21:47:32Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot · 2022-08-13T22:37:03Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen
Mark this issue or PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2022-08-13T22:37:23Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen

Mark this issue or PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

upodroid · 2022-08-15T12:20:29Z

/reopen

k8s-ci-robot · 2022-08-15T12:20:46Z

@upodroid: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2022-08-15T12:21:11Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: upodroid
Once this PR has been reviewed and has the lgtm label, please ask for approval from cblecker by writing /assign @cblecker in a comment. For more information see:The Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

groups/OWNERS

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

k8s-ci-robot · 2022-08-23T05:36:53Z

@upodroid: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dims · 2022-08-23T12:36:08Z

let's close this for now. it's not getting eyes and it needs a rebase anyway as many things have changed.

rewrite the reconciler

0bce68c

k8s-ci-robot requested a review from dims February 16, 2022 15:10

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Feb 16, 2022

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 16, 2022

k8s-ci-robot assigned cblecker Feb 16, 2022

evankanderson reviewed Feb 17, 2022

View reviewed changes

switch to runE and start returning errors

064b12d

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 9, 2022

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 10, 2022

apply Evan's recommendations

7908ba4

upodroid force-pushed the rewrite-reconciler branch from e845219 to 7908ba4 Compare March 10, 2022 17:04

Merge branch 'main' of github.com:kubernetes/k8s.io into rewrite-reco…

1067c84

…nciler

add headers

452a944

add new make targets

b97545f

upodroid mentioned this pull request Mar 10, 2022

Update GSuite Reconciler prow jobs kubernetes/test-infra#25582

Closed

upodroid mentioned this pull request May 19, 2022

REQUEST: New membership for upodroid kubernetes/org#3429

Closed

9 tasks

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 8, 2022

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 14, 2022

k8s-ci-robot closed this Aug 13, 2022

k8s-ci-robot reopened this Aug 15, 2022

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 23, 2022

dims closed this Aug 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the GSuite Reconciler #3407

Refactor the GSuite Reconciler #3407

upodroid commented Feb 16, 2022

k8s-ci-robot commented Feb 16, 2022

linux-foundation-easycla bot commented Feb 16, 2022 •

edited

k8s-ci-robot commented Feb 16, 2022

k8s-ci-robot commented Feb 16, 2022

dims commented Feb 16, 2022

evankanderson left a comment

evankanderson Feb 17, 2022

evankanderson Feb 17, 2022

evankanderson Feb 17, 2022

upodroid Feb 22, 2022

evankanderson Feb 17, 2022

dims commented Mar 9, 2022

upodroid commented Mar 9, 2022

puerco commented Mar 10, 2022

upodroid commented Mar 10, 2022 •

edited

k8s-triage-robot commented Jun 8, 2022

evankanderson commented Jun 14, 2022

k8s-triage-robot commented Jul 14, 2022

k8s-triage-robot commented Aug 13, 2022

k8s-ci-robot commented Aug 13, 2022

upodroid commented Aug 15, 2022

k8s-ci-robot commented Aug 15, 2022

k8s-ci-robot commented Aug 15, 2022

k8s-ci-robot commented Aug 23, 2022

dims commented Aug 23, 2022

Refactor the GSuite Reconciler #3407

Refactor the GSuite Reconciler #3407

Conversation

upodroid commented Feb 16, 2022

k8s-ci-robot commented Feb 16, 2022

linux-foundation-easycla bot commented Feb 16, 2022 • edited

k8s-ci-robot commented Feb 16, 2022

k8s-ci-robot commented Feb 16, 2022

dims commented Feb 16, 2022

evankanderson left a comment

Choose a reason for hiding this comment

evankanderson Feb 17, 2022

Choose a reason for hiding this comment

evankanderson Feb 17, 2022

Choose a reason for hiding this comment

evankanderson Feb 17, 2022

Choose a reason for hiding this comment

upodroid Feb 22, 2022

Choose a reason for hiding this comment

evankanderson Feb 17, 2022

Choose a reason for hiding this comment

dims commented Mar 9, 2022

upodroid commented Mar 9, 2022

puerco commented Mar 10, 2022

upodroid commented Mar 10, 2022 • edited

k8s-triage-robot commented Jun 8, 2022

evankanderson commented Jun 14, 2022

k8s-triage-robot commented Jul 14, 2022

k8s-triage-robot commented Aug 13, 2022

k8s-ci-robot commented Aug 13, 2022

upodroid commented Aug 15, 2022

k8s-ci-robot commented Aug 15, 2022

k8s-ci-robot commented Aug 15, 2022

k8s-ci-robot commented Aug 23, 2022

dims commented Aug 23, 2022

linux-foundation-easycla bot commented Feb 16, 2022 •

edited

upodroid commented Mar 10, 2022 •

edited