Refactor auth client code into lib/auth/authclient #41460
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rosstimothy
added
backport/branch/v13
backport/branch/v14
no-changelog
rosstimothy
force-pushed
the
tross/authclient
branch
from
e8e73c6
to
966fb45
Compare
|
@zmb3 can I get a FTD bypass on TestClient_DialTimeout? |
|
/excludeflake TestClient_DialTimeout |
This starts separating the lib/auth client and server code into distinct packages to reduce the dependency tree of client tools. For the time being aliases were left to all things moved into lib/auth/authclient to reduce the changeset and ensure code still compiles. Future changes will be built off this to convert code to use lib/auth/authclient where applicable, this was intentionally left to a minimum amount of changes to facilitate backports.
rosstimothy
force-pushed
the
tross/authclient
branch
from
966fb45
to
5ebed30
Compare
strideynet
approved these changes
May 13, 2024
| // the proxy. | ||
| Resolver reversetunnelclient.Resolver | ||
| // ProxyDialer is used to create a client via a Proxy reverse tunnel. | ||
| ProxyDialer apiclient.ContextDialer |
There was a problem hiding this comment.
Might be worth adding a note to this context to the effect of "For example, reverseclient.TunnelAuthDialer"
| } | ||
|
|
||
| // Connect creates a valid client connection to the auth service. It may | ||
| // connect directly to the auth server, or tunnel through the proxy. | ||
| func Connect(ctx context.Context, cfg *Config) (*auth.Client, error) { | ||
| func Connect(ctx context.Context, cfg *Config) (*Client, error) { |
There was a problem hiding this comment.
I think likely out of the scope of this PR, but this function has felt quite weird to me in the past, and feels even weirder now. If I know what ProxyDialer I want to use, it's weird that this tries connecting to the auth server directly first. Perhaps a more long-term thing, but it often feels to me that what we really want is something like:
direct := NewDirectDialler()
reverseTunnel := NewReverseTunnelDialler()
client := NewClient(TryInOrderStrategy(direct, reverseTunnel))
// or
client := NewClient(RacingStrategy(direct, reverseTunnel))I have this same thought about the api.Client as well - so perhaps we just await the death of the auth client and then at some-point in the future we refactor api.Client's dialling mechanism to be less odd.
There was a problem hiding this comment.
Yeah I don't disagree the usefulness of this function is now at an all time low. Alternatively I can refactor a few things in reversectunnelclient so that it doesn't import lib/auth or lib/authclient and create a cycle.
There was a problem hiding this comment.
I think we can leave this as is for now - we probably want to look at this as an individual thing I imagine.
greedy52
approved these changes
May 14, 2024
|
@rosstimothy See the table below for backport results.
|
rosstimothy
added a commit
that referenced
this pull request
May 14, 2024
This starts separating the lib/auth client and server code into distinct packages to reduce the dependency tree of client tools. For the time being aliases were left to all things moved into lib/auth/authclient to reduce the changeset and ensure code still compiles. Future changes will be built off this to convert code to use lib/auth/authclient where applicable, this was intentionally left to a minimum amount of changes to facilitate backports.
rosstimothy
added a commit
that referenced
this pull request
May 14, 2024
This starts separating the lib/auth client and server code into distinct packages to reduce the dependency tree of client tools. For the time being aliases were left to all things moved into lib/auth/authclient to reduce the changeset and ensure code still compiles. Future changes will be built off this to convert code to use lib/auth/authclient where applicable, this was intentionally left to a minimum amount of changes to facilitate backports.
rosstimothy
added a commit
that referenced
this pull request
May 14, 2024
This starts separating the lib/auth client and server code into distinct packages to reduce the dependency tree of client tools. For the time being aliases were left to all things moved into lib/auth/authclient to reduce the changeset and ensure code still compiles. Future changes will be built off this to convert code to use lib/auth/authclient where applicable, this was intentionally left to a minimum amount of changes to facilitate backports.
ptgott
pushed a commit
that referenced
this pull request
May 14, 2024
This starts separating the lib/auth client and server code into distinct packages to reduce the dependency tree of client tools. For the time being aliases were left to all things moved into lib/auth/authclient to reduce the changeset and ensure code still compiles. Future changes will be built off this to convert code to use lib/auth/authclient where applicable, this was intentionally left to a minimum amount of changes to facilitate backports.
rosstimothy
added a commit
that referenced
this pull request
May 14, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
rosstimothy
added a commit
that referenced
this pull request
May 14, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 14, 2024
This starts separating the lib/auth client and server code into distinct packages to reduce the dependency tree of client tools. For the time being aliases were left to all things moved into lib/auth/authclient to reduce the changeset and ensure code still compiles. Future changes will be built off this to convert code to use lib/auth/authclient where applicable, this was intentionally left to a minimum amount of changes to facilitate backports.
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 14, 2024
This starts separating the lib/auth client and server code into distinct packages to reduce the dependency tree of client tools. For the time being aliases were left to all things moved into lib/auth/authclient to reduce the changeset and ensure code still compiles. Future changes will be built off this to convert code to use lib/auth/authclient where applicable, this was intentionally left to a minimum amount of changes to facilitate backports.
rosstimothy
added a commit
that referenced
this pull request
May 15, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 15, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
rosstimothy
added a commit
that referenced
this pull request
May 15, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
rosstimothy
added a commit
that referenced
this pull request
May 15, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
rosstimothy
added a commit
that referenced
this pull request
May 15, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
rosstimothy
added a commit
that referenced
this pull request
May 15, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
rosstimothy
added a commit
that referenced
this pull request
May 15, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
rosstimothy
added a commit
that referenced
this pull request
May 15, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 15, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 16, 2024
Consumes the new auth client created in #41460. The changes introduced were performed with a series of gofmt rewrite rules. ```bash $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "ClientI -> authclient.ClientI" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "Client -> authclient.Client" {} \; $ find ./lib/auth -type f -name '*.go' -exec gofmt -w -l -r "NewClient -> authclient.NewClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.ClientI -> authclient.ClientI" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.Client -> authclient.Client" {} \; $ find . -type f -name '*.go' -exec gofmt -w -l -r "auth.NewClient -> authclient.NewClient" {} \; ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This starts separating the lib/auth client and server code into distinct packages to reduce the dependency tree of client tools. For the time being aliases were left to all things moved into lib/auth/authclient to reduce the changeset and ensure code still compiles. Future changes will be built off this to convert code to use lib/auth/authclient where applicable, this was intentionally left to a minimum amount of changes to facilitate backports.