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

Cloud #2998

Merged
merged 20 commits into from May 13, 2024
Merged

Cloud #2998

merged 20 commits into from May 13, 2024

Conversation

markphelps
Copy link
Collaborator

@markphelps markphelps commented Apr 18, 2024

Draft PR for cloud/tunnel support for Flipt Hybrid Cloud

We will use this PR as the base for all cloud functionality in Flipt, branching and merging additional PRs off of this branch as we go.

cloud:
  tunnel:
    address: "flipt.cloud" # defaults to this so most folks don't care (but we can override)
    port: 8443 # defaults to this so most folks don't care
    organization: foo
    instance: bar
  authentication:
    required: true
    api_key: xyz
  audit:
    enabled: true

@GeorgeMac
Copy link
Contributor

While there is a part of me that feels like littering "cloud" term in multiple places feels bad, I think it might be better for the dependencies to organize it in this way. Otherwise, we're drilling down new config sections.

So long story short, I like the first and what you have in this PR, I think.
We can have folks piecemeal opt into features they want managed in cloud, in this way.

Open to pushback though.

cmd/flipt/main.go Outdated Show resolved Hide resolved
@markphelps
Copy link
Collaborator Author

While there is a part of me that feels like littering "cloud" term in multiple places feels bad, I think it might be better for the dependencies to organize it in this way. Otherwise, we're drilling down new config sections.

So long story short, I like the first and what you have in this PR, I think. We can have folks piecemeal opt into features they want managed in cloud, in this way.

Open to pushback though.

yeah after seeing it written out like this I think I agree. Initially I thought I would like the latter approach, but now I prefer the former. we could still potentially 'guard' everything under experimental with a single key like:

experimental
  cloud: true

which only if true would mean that the other configs are respected. wdyt?

@erka
Copy link
Collaborator

erka commented Apr 18, 2024

@markphelps Hard to say without good insights.

I think there should not be authentication section. api_key could be in the same section as address. I would suggest to have address as company.flipt.cloud and don't have extra fields in configuration. It may be simpler for folks to put one line/env vs few ones.

chore: add config tests and auth

chore: add api key for tunnel connection

chore: config changes

chore: fix server close
markphelps and others added 17 commits April 30, 2024 14:37
* feat: start impl of cloud link

* chore: change token param to id_token

Signed-off-by: Mark Phelps <[email protected]>

* chore: start http server

* chore: PR updates

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix write html

Signed-off-by: Mark Phelps <[email protected]>

* chore: rm uneeded struct

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix cue schema

Signed-off-by: Mark Phelps <[email protected]>

* chore: respond to PR feedback, fix schema

* chore: try to fix cli ITs

* chore: set min TLS version

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
* feat: start cloud serve command

* chore: start flipt with new config

* feat: validate JWT; retry reverst connections; check for expiry and existing instance

* chore: handle error response

* chore: add spinner

* Revert "chore: add spinner"

This reverts commit bb35379.

* chore: dont make org/instance/api key required for now

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix tests

Signed-off-by: Mark Phelps <[email protected]>

* feat: impl cloud auth (#3045)

* feat: impl cloud auth

* chore: PR feedback

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix tests

---------

Signed-off-by: Mark Phelps <[email protected]>

* chore: use backoff from reverst

* chore: fix build

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: George MacRorie <[email protected]>
* chore: check for existing instance in cloud

* chore: close body

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>
* main:
  chore(deps): bump golangci/golangci-lint-action from 4.0.0 to 5.3.0 (#3059)
* feat: add cloud audit sink

* chore: cant enforce auth always for cloud

Signed-off-by: Mark Phelps <[email protected]>

* feat: make cloud experimental

* chore: fix schema

* chore: try to fix tests

Signed-off-by: Mark Phelps <[email protected]>

* chore: fmt audit payload

Signed-off-by: Mark Phelps <[email protected]>

* chore: basic cloud audit event test

Signed-off-by: Mark Phelps <[email protected]>

* chore: telemetry test

Signed-off-by: Mark Phelps <[email protected]>

* chore: change serve command desc

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps marked this pull request as ready for review May 10, 2024 18:21
@markphelps markphelps requested a review from a team as a code owner May 10, 2024 18:21
@markphelps markphelps requested a review from GeorgeMac May 10, 2024 18:21
Comment on lines +132 to +182
g.Go(func() error {
tok, err := flow.Wait(ctx)
if err != nil {
return fmt.Errorf("waiting for token: %w", err)
}

cloudAuth := cloudAuth{
Token: tok,
}

cloudAuthBytes, err := json.Marshal(cloudAuth)
if err != nil {
return fmt.Errorf("marshalling cloud auth token: %w", err)
}

if err := os.WriteFile(cloudAuthFile, cloudAuthBytes, 0600); err != nil {
return fmt.Errorf("writing cloud auth token: %w", err)
}

fmt.Println("\n✓ Authenticated with Flipt Cloud!\nYou can now run commands that require cloud authentication.")

return nil
})

g.Go(func() error {
if err := flow.StartServer(nil); err != nil && !errors.Is(err, net.ErrClosed) {
return fmt.Errorf("starting server: %w", err)
}
close(done)
return nil
})

g.Go(func() error {
select {
case <-done:
cancel()
case <-ctx.Done():
if err := flow.Close(); err != nil && !errors.Is(err, context.Canceled) {
return err
}
}
return nil
})

g.Go(func() error {
url, err := flow.BrowserURL(fmt.Sprintf("%s/login/device", c.url))
if err != nil {
return fmt.Errorf("creating browser URL: %w", err)
}
return util.OpenBrowser(url)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Just making a note for future refactoring: lots of goroutines here, which I think have little extra value.
I think we need only one for the blocking server start and then the rest could probably be synchronous with this command.

  1. go start server
  2. open browser (looks non-blocking, is that right?)
  3. flow.Wait which observes context cancelled
  4. observe token and write out file

Copy link
Contributor

@GeorgeMac GeorgeMac left a comment

Choose a reason for hiding this comment

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

👏 Nice

@markphelps markphelps merged commit 4bc8bf7 into main May 13, 2024
31 checks passed
@markphelps markphelps deleted the cloud branch May 13, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants