Public API Separation - #476
Merged
Merged
Conversation
Previously, an api package was added, but it also put the
implementation there. This causes package cycle problems
and subverts the idea of an API.
This migrates `api.Run` (impl) to `api.RunFunc` (api) and
`func_e.Run` (impl). This also backfills a test to make
sure the code works.
To do this, we migrate `/main.go` -> `/cmd/func-e/main.go`,
which allows us to re-use the root package as a library,
instead of a binary (main) one.
There is impact, but to only one user: envoy gateway. The
fix is mechanical and allows us to clear tech debt before
making more features.
```diff
diff --git a/internal/infrastructure/host/proxy_infra.go b/internal/infrastructure/host/proxy_infra.go
index f384b6854..229fdcd1c 100644
--- a/internal/infrastructure/host/proxy_infra.go
+++ b/internal/infrastructure/host/proxy_infra.go
@@ -12,7 +12,8 @@ import (
"os"
"path/filepath"
- funcE "github.com/tetratelabs/func-e/api"
+ func_e "github.com/tetratelabs/func-e"
+ "github.com/tetratelabs/func-e/api"
"k8s.io/utils/ptr"
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
@@ -92,7 +93,7 @@ func (i *Infra) runEnvoy(ctx context.Context, out io.Writer, name string, args [
defer func() {
exit <- struct{}{}
}()
- err := funcE.Run(pCtx, args, funcE.HomeDir(i.HomeDir), funcE.Out(out))
+ err := func_e.Run(pCtx, args, api.HomeDir(i.HomeDir), api.Out(out))
if err != nil {
i.Logger.Error(err, "failed to run envoy")
}
diff --git a/internal/infrastructure/host/proxy_infra_test.go b/internal/infrastructure/host/proxy_infra_test.go
index 2d82cc689..36bc997f0 100644
--- a/internal/infrastructure/host/proxy_infra_test.go
+++ b/internal/infrastructure/host/proxy_infra_test.go
@@ -13,7 +13,8 @@ import (
"testing"
"github.com/stretchr/testify/require"
- funcE "github.com/tetratelabs/func-e/api"
+ func_e "github.com/tetratelabs/func-e"
+ "github.com/tetratelabs/func-e/api"
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/internal/crypto"
@@ -91,7 +92,7 @@ func TestInfraCreateProxy(t *testing.T) {
func TestInfra_runEnvoy_stopEnvoy(t *testing.T) {
tmpdir := t.TempDir()
// Ensures that all the required binaries are available.
- err := funcE.Run(context.Background(), []string{"--version"}, funcE.HomeDir(tmpdir))
+ err := func_e.Run(context.Background(), []string{"--version"}, api.HomeDir(tmpdir))
require.NoError(t, err)
i := &Infra{proxyContextMap: make(map[string]*proxyContext), HomeDir: tmpdir}
```
Signed-off-by: Adrian Cole <adrian@tetrate.io>
| // | ||
| // Note: None of these default to values read from OS environment variables. | ||
| // If you wish to introduce such behavior, populate them in calling code. | ||
| type RunOption func(*opts.RunOpts) |
Contributor
Author
There was a problem hiding this comment.
this uses an internal type which is a trick to effectively hide the exported fields
| ) | ||
|
|
||
| // fakeFuncEFactory implements runtest.FuncEFactory for API tests using fake envoy | ||
| type fakeFuncEFactory struct{} | ||
|
|
||
| func (fakeFuncEFactory) New(ctx context.Context, t *testing.T, stdout, stderr io.Writer) (e2e.FuncE, error) { | ||
| // Start and scope a fake version server to the test calling New | ||
| versionsServer := test.RequireEnvoyVersionsTestServer(t, version.LastKnownEnvoy) |
Contributor
Author
There was a problem hiding this comment.
versionsServer is ignored when you supply the internal-only envoyPath, so this makes the tests run slightly quicker
codefromthecrypt
left a comment
Contributor
Author
There was a problem hiding this comment.
note: I don't think RunFunc is the best api, but it allows a mechanical update to existing envoy gateway code. We can make another better (second) API once things are updated, provided we don't put impl in the api package again.
Contributor
Author
|
once this is merged and after next release we need to fix homebrew to |
mathetake
approved these changes
Jul 21, 2025
This file contains hidden or 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
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.
Previously, an api package was added, but it also put the implementation there. This causes package cycle problems and subverts the idea of an API.
This migrates
api.Run(impl) toapi.RunFunc(api) andfunc_e.Run(impl). This also backfills a test to make sure the code works.To do this, we migrate
/main.go->/cmd/func-e/main.go, which allows us to re-use the root package as a library, instead of a binary (main) one.There is impact, but to only one user: envoy gateway. The fix is mechanical and allows us to clear tech debt before making more features.