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

move snapshots interface to localhost #10302

Merged
merged 14 commits into from
Nov 12, 2024
Merged

move snapshots interface to localhost #10302

merged 14 commits into from
Nov 12, 2024

Conversation

yuval-k
Copy link
Contributor

@yuval-k yuval-k commented Nov 8, 2024

move the snapshots debug endpoints to a localhost only admin interface

@github-actions github-actions bot added keep pr updated signals bulldozer to keep pr up to date with base branch work in progress signals bulldozer to keep pr open (don't auto-merge) labels Nov 8, 2024
Copy link

github-actions bot commented Nov 8, 2024

Visit the preview URL for this PR (updated for commit 1b27f44):

https://gloo-edge--pr10302-yuval-k-snapshot-loc-2i93ab1z.web.app

(expires Tue, 19 Nov 2024 06:42:51 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

@yuval-k yuval-k enabled auto-merge (squash) November 8, 2024 20:05
Copy link
Contributor

@sam-heilbron sam-heilbron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

What's the long-term plan for that env variable that controls the behavior? Is it intended as a short-terms backwards compatibility?

projects/gloo/pkg/syncer/setup/start_func.go Outdated Show resolved Hide resolved
projects/gloo/pkg/syncer/setup/start_func.go Show resolved Hide resolved
projects/gloo/pkg/syncer/setup/start_func.go Outdated Show resolved Hide resolved
projects/gloo/pkg/syncer/setup/start_func.go Outdated Show resolved Hide resolved
projects/gloo/pkg/syncer/setup/start_func.go Outdated Show resolved Hide resolved
@yuval-k
Copy link
Contributor Author

yuval-k commented Nov 11, 2024

What's the long-term plan for that env variable that controls the behavior? Is it intended as a short-terms backwards compatibility?

Yes, i left it there just in case someone depends on the old functionality. if no one complains delete in 1.19

@sam-heilbron sam-heilbron removed the work in progress signals bulldozer to keep pr open (don't auto-merge) label Nov 11, 2024
"github.com/solo-io/go-utils/threadsafe"
)

const (
InputSnapshotPath = "/snapshots/input"
DefaultAdminPort = 9091

DefaultAdminPort = admin.AdminPort
Copy link
Contributor

@bleggett bleggett Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd almost say drop this export?

It's used here and in one (1) test, and in that test might as well import "github.com/solo-io/gloo/projects/gloo/pkg/servers/admin"

Not a big deal tho.

@yuval-k yuval-k merged commit 2d5e231 into main Nov 12, 2024
19 checks passed
@yuval-k yuval-k deleted the yuval-k/snapshot-localhost branch November 12, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep pr updated signals bulldozer to keep pr up to date with base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants