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.

Sign up for GitHub

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

Jump to bottom

Untangle genproto / cloud.google.com/go dependencies #113366

Open
16 of 38 tasks
liggitt opened this issue Oct 26, 2022 · 10 comments · May be fixed by #128419
Open
16 of 38 tasks

Untangle genproto / cloud.google.com/go dependencies #113366

liggitt opened this issue Oct 26, 2022 · 10 comments · May be fixed by #128419
Assignees
@liggitt
Labels
area/code-organization Issues or PRs related to kubernetes code organization lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture.

Comments

@liggitt
Copy link
Member

liggitt commented Oct 26, 2022 •
edited
Loading

#113225 uncovered a coming snarl of dependencies between cloud.google.com/go and google.golang.org/genproto

Because several generic libraries use the cloud.google.com/go/compute/metadata package, the following chain makes it almost impossible to avoid adding all cloud.google.com/go submodules into the dependency graph for all downstream consumers:

  • {golang.org/x/oauth2, google.golang.org/api, google.golang.org/grpc} --> cloud.google.com/go/compute --> cloud.google.com/go --> google.golang.org/genproto --> cloud.google.com/go/*

It's possible we can avoid the updates that expanded these cross-references in the short-term with judicious upstream changes back to older genproto versions, but we should get ahead of this if we want to avoid being forced to pull in these dependency graph increases in the future for a critical bugfix.

Recommendations

1. Isolate cloud.google.com/go/compute/metadata

The cloud.google.com/go/compute/metadata package (https://github.com/googleapis/google-cloud-go/tree/main/compute/metadata) only uses stdlib dependencies, but lives inside cloud.google.com/go/compute, which has many other references (https://github.com/googleapis/google-cloud-go/blob/main/compute/go.mod)

  1. Isolate cloud.google.com/go/compute/metadata to its own zero-dependency module

  2. Sweep dependencies using cloud.google.com/go/compute/metadata and update them to use the zero-dependency alternative

    • golang.org/x/oauth2 - v0.20.0+
    • google.golang.org/api - v0.174.0+
    • google.golang.org/grpc - v1.65.0+
    • github.com/google/cadvisor (transitive via golang.org/x/oauth2, google.golang.org/api, google.golang.org/grpc)
    • go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc (transitive via golang.org/x/oauth2, google.golang.org/grpc)
      • committed
      • tagged

2. Prune use of cloud genproto packages in non-cloud.google.com modules

  1. Update github.com/googleapis/gax-go/v2 test to not require the google.golang.org/genproto/googleapis/cloud/location package

3. Drop use of google.golang.org/genproto/protobuf/field_mask

Update to use canonical google.golang.org/protobuf/types/known/fieldmaskpb package instead

4. Isolate generic genproto packages

/priority important-longterm
/area code-organization
/sig architecture
/cc @dims
fyi @codyoss @bertinatto
@k8s-ci-robot k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. area/code-organization Issues or PRs related to kubernetes code organization sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 26, 2022
@liggitt liggitt self-assigned this Oct 26, 2022
@liggitt liggitt added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Oct 26, 2022
@liggitt liggitt mentioned this issue Oct 26, 2022
Merged
@codyoss
Copy link

codyoss commented Oct 26, 2022

I am in the process of doing item one rn: googleapis/google-cloud-go#6311

@codyoss
Copy link

codyoss commented Oct 26, 2022

For item three I might say this can be simplified to the following:

  • google.golang.org/genproto/googleapis/api/*
  • google.golang.org/genproto/googleapis/rpc/*
  • google.golang.org/genproto/googleapis/type/* (new entry)
  • google.golang.org/genproto/googleapis/bytestream

google.golang.org/genproto/protobuf/field_mask is already an alias to another project that should be used directly.
It may be easiest to create a new module somewhere to house all of these super general stubs. Then genproto just alias to that repo.

@codyoss
Copy link

codyoss commented Oct 26, 2022

cc @noahdietz @quartzmo

@liggitt liggitt added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Oct 26, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Oct 26, 2022
@liggitt liggitt mentioned this issue Oct 26, 2022
Merged
@liggitt
Copy link
Member Author

liggitt commented Oct 26, 2022

opened googleapis/gax-go#237 for item 2

This was referenced Oct 26, 2022
Merged
Closed
Merged
@aramase aramase mentioned this issue Nov 4, 2022
Closed
@aramase
Copy link
Member

aramase commented Nov 4, 2022 •
edited
Loading

opened google/cadvisor#3194 for github.com/google/cadvisor . I've marked it as WIP so we can update the transitive deps in 1 PR.

@liggitt liggitt moved this from Triage to Dependencies to improve in [sig-architecture] Dependency management Nov 10, 2022
@skitt skitt mentioned this issue Jan 3, 2023
Closed
skitt added a commit to skitt/go-oidc that referenced this issue Jan 3, 2023
@skitt
Bump golang.org/x dependencies
aa01b42 
The latest versions simplify the transitive dependency tree (see
kubernetes/kubernetes#113366 for details).

Signed-off-by: Stephen Kitt <skitt@redhat.com>
@skitt skitt mentioned this issue Jan 3, 2023
Merged
ericchiang pushed a commit to coreos/go-oidc that referenced this issue Jan 3, 2023
@skitt @ericchiang
Bump golang.org/x dependencies
a8ceb9a 
The latest versions simplify the transitive dependency tree (see
kubernetes/kubernetes#113366 for details).

Signed-off-by: Stephen Kitt <skitt@redhat.com>
@liggitt liggitt mentioned this issue Jan 4, 2023
Merged
@codyoss codyoss mentioned this issue Jan 4, 2023
Closed
skitt added a commit to skitt/opentelemetry-go-contrib that referenced this issue Jan 4, 2023
@skitt
Bump grpc to 1.53
16d4351 
This drops the indirect dependency on cloud.google.com/go, which helps
reduce the dependency tree (see
kubernetes/kubernetes#113366 and
kubernetes/kubernetes#114772 for the wider
context).

Signed-off-by: Stephen Kitt <skitt@redhat.com>
@liggitt liggitt added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label May 26, 2023
quartzmo added a commit to quartzmo/google-cloud-go that referenced this issue Jun 1, 2023
@quartzmo
fix(pubsublite): Use fieldmask directly instead of field_mask genprot…
474efdb 
…o alias

related to kubernetes/kubernetes#113366
@quartzmo quartzmo mentioned this issue Jun 1, 2023
Merged
quartzmo added a commit to quartzmo/google-cloud-go that referenced this issue Jun 1, 2023
@quartzmo
fix(pubsub): Use fieldmask directly instead of field_mask genproto alias
d27761c 
related to kubernetes/kubernetes#113366
@quartzmo quartzmo mentioned this issue Jun 1, 2023
Merged
quartzmo added a commit to quartzmo/google-cloud-go that referenced this issue Jun 1, 2023
@quartzmo
fix(logging): Use fieldmask directly instead of field_mask genproto a…
711489a 
…lias

related to kubernetes/kubernetes#113366
@quartzmo quartzmo mentioned this issue Jun 1, 2023
Merged
quartzmo added a commit to quartzmo/google-cloud-go that referenced this issue Jun 1, 2023
@quartzmo
fix(bigtable): Use fieldmask directly instead of field_mask genproto …
8a740f0 
…alias

related to kubernetes/kubernetes#113366
@quartzmo quartzmo mentioned this issue Jun 1, 2023
Merged
quartzmo added a commit to quartzmo/rules_go that referenced this issue Jun 1, 2023
@quartzmo
Update go.mod github.com/golang/protobuf and github.com/golang/genproto
61b583a 
related to kubernetes/kubernetes#113366
quartzmo added a commit to googleapis/google-cloud-go that referenced this issue Jun 6, 2023
@quartzmo
fix(pubsublite): Use fieldmask directly instead of field_mask genprot…
30a196b 
…o alias (#8029)

related to kubernetes/kubernetes#113366
gcf-merge-on-green bot pushed a commit to googleapis/google-cloud-go that referenced this issue Jun 6, 2023
@quartzmo
fix(bigtable): Use fieldmask directly instead of field_mask genproto …
cae6cd6 
…alias (#8032)

related to kubernetes/kubernetes#113366
quartzmo added a commit to googleapis/google-cloud-go that referenced this issue Jun 6, 2023
@quartzmo
fix(pubsub): Use fieldmask directly instead of field_mask genproto al…
087a5fc 
…ias (#8030)

related to kubernetes/kubernetes#113366
gcf-merge-on-green bot pushed a commit to googleapis/google-cloud-go that referenced this issue Jun 6, 2023
@quartzmo
fix(logging): Use fieldmask directly instead of field_mask genproto a…
13d9483 
…lias (#8031)

related to kubernetes/kubernetes#113366
@dgrisonnet dgrisonnet mentioned this issue Jun 15, 2023
Merged
@MartinForReal MartinForReal mentioned this issue Jun 30, 2023
Closed
@liggitt liggitt mentioned this issue Sep 1, 2023
Closed
@liggitt liggitt mentioned this issue Feb 28, 2024
Closed
lukaszraczylo pushed a commit to lukaszraczylo/go-oidc that referenced this issue Apr 13, 2024
@skitt @lukaszraczylo
Bump golang.org/x dependencies
f23f3ae 
The latest versions simplify the transitive dependency tree (see
kubernetes/kubernetes#113366 for details).

Signed-off-by: Stephen Kitt <skitt@redhat.com>
@liggitt liggitt mentioned this issue Jun 19, 2024
Merged
dashpole added a commit to dashpole/kubernetes that referenced this issue Jun 23, 2024
@dashpole
update unwanted dependencies
d65029f 
This update dropped the otelgrpc → cloud.google.com/go/compute dependency,
among others. This dropped out because genproto cleaned up it's dependencies
on google cloud libraries, and otel updated - details in kubernetes#113366.
vinayakankugoyal pushed a commit to vinayakankugoyal/kubernetes that referenced this issue Jun 28, 2024
@dashpole @vinayakankugoyal
update unwanted dependencies
3bbc27a 
This update dropped the otelgrpc → cloud.google.com/go/compute dependency,
among others. This dropped out because genproto cleaned up it's dependencies
on google cloud libraries, and otel updated - details in kubernetes#113366.
harpreet9ja pushed a commit to harpreet9ja/kubernetes that referenced this issue Jun 30, 2024
@dashpole @harpreet9ja
update unwanted dependencies
3a948c6 
This update dropped the otelgrpc → cloud.google.com/go/compute dependency,
among others. This dropped out because genproto cleaned up it's dependencies
on google cloud libraries, and otel updated - details in kubernetes#113366.
dims pushed a commit to dims/kubernetes that referenced this issue Jul 5, 2024
@dashpole @dims
update unwanted dependencies
0a7af0b 
This update dropped the otelgrpc → cloud.google.com/go/compute dependency,
among others. This dropped out because genproto cleaned up it's dependencies
on google cloud libraries, and otel updated - details in kubernetes#113366.

Signed-off-by: Davanum Srinivas <davanum@gmail.com>
dims added a commit to dims/kubernetes that referenced this issue Jul 6, 2024
@dims @dashpole
update OpenTelemetry dependencies and grpc
ff7942b 
This update dropped the otelgrpc → cloud.google.com/go/compute dependency,
among others. This dropped out because genproto cleaned up it's dependencies
on google cloud libraries, and otel updated - details in kubernetes#113366.

Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Co-Authored-By: David Ashpole <dashpole@google.com>
@liggitt liggitt mentioned this issue Jul 16, 2024
Closed
YamasouA pushed a commit to YamasouA/kubernetes that referenced this issue Jul 24, 2024
@dashpole @YamasouA
update unwanted dependencies
806733b 
This update dropped the otelgrpc → cloud.google.com/go/compute dependency,
among others. This dropped out because genproto cleaned up it's dependencies
on google cloud libraries, and otel updated - details in kubernetes#113366.
YamasouA pushed a commit to YamasouA/kubernetes that referenced this issue Jul 24, 2024
@dims @dashpole @YamasouA
update OpenTelemetry dependencies and grpc
78e2d23 
This update dropped the otelgrpc → cloud.google.com/go/compute dependency,
among others. This dropped out because genproto cleaned up it's dependencies
on google cloud libraries, and otel updated - details in kubernetes#113366.

Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Co-Authored-By: David Ashpole <dashpole@google.com>
This was referenced Oct 29, 2024
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@liggitt liggitt

Labels
area/code-organization Issues or PRs related to kubernetes code organization lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture.
Projects
[sig-architecture] Dependency management
Status: Dependencies to improve
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

WIP: dry-run etcd 3.6 dependency update liggitt/kubernetes
8 participants
@liggitt @pacoxu @dashpole @codyoss @aramase @nikhita @k8s-ci-robot @k8s-triage-robot