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

unrevert "tiltfile: disallow deploys to remote kube by default" #2012

Merged
merged 3 commits into from
Aug 8, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions internal/cli/wire_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion internal/engine/upper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
"github.com/windmilleng/tilt/internal/synclet"
"github.com/windmilleng/tilt/internal/testutils"
"github.com/windmilleng/tilt/internal/testutils/bufsync"
"github.com/windmilleng/tilt/internal/testutils/k8sutils"
"github.com/windmilleng/tilt/internal/testutils/manifestbuilder"
"github.com/windmilleng/tilt/internal/testutils/podbuilder"
"github.com/windmilleng/tilt/internal/testutils/tempdir"
Expand Down Expand Up @@ -2686,7 +2687,9 @@ func newTestFixture(t *testing.T) *testFixture {

fakeDcc := dockercompose.NewFakeDockerComposeClient(t, ctx)

tfl := tiltfile.ProvideTiltfileLoader(ta, k8s, fakeDcc, "fake-context", feature.MainDefaults)
k8sConfig := k8sutils.NewConfig("fake-context", "fake-cluster", "http://localhost:6443")

tfl := tiltfile.ProvideTiltfileLoader(ta, k8s, fakeDcc, "fake-context", k8sConfig, feature.MainDefaults)
cc := NewConfigsController(tfl, dockerClient)
dcw := NewDockerComposeEventWatcher(fakeDcc)
dclm := NewDockerComposeLogManager(fakeDcc)
Expand Down
15 changes: 15 additions & 0 deletions internal/testutils/k8sutils/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package k8sutils

import "k8s.io/client-go/tools/clientcmd/api"

func NewConfig(contextName string, clusterName string, server string) *api.Config {
ret := api.NewConfig()
ret.CurrentContext = contextName
context := api.NewContext()
context.Cluster = clusterName
ret.Contexts[contextName] = context
cluster := api.NewCluster()
cluster.Server = server
ret.Clusters[clusterName] = cluster
return ret
}
70 changes: 66 additions & 4 deletions internal/tiltfile/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,23 @@ package tiltfile

import (
"fmt"
"net/url"
"regexp"
"sort"
"strconv"
"strings"

"github.com/windmilleng/tilt/internal/sliceutils"

"go.starlark.net/syntax"

"github.com/docker/distribution/reference"
"github.com/pkg/errors"
"go.starlark.net/starlark"
"go.starlark.net/syntax"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/tools/clientcmd/api"

"github.com/windmilleng/tilt/internal/container"
"github.com/windmilleng/tilt/internal/k8s"
"github.com/windmilleng/tilt/internal/model"
"github.com/windmilleng/tilt/internal/sliceutils"
)

type referenceList []reference.Named
Expand Down Expand Up @@ -887,3 +888,64 @@ func newK8sObjectID(e k8s.K8sEntity) k8sObjectID {
func (s *tiltfileState) k8sContext(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
return starlark.String(s.kubeContext), nil
}

func (s *tiltfileState) allowK8SContexts(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
var contexts starlark.Value
if err := starlark.UnpackArgs(fn.Name(), args, kwargs,
"contexts", &contexts,
); err != nil {
return nil, err
}

for _, c := range starlarkValueOrSequenceToSlice(contexts) {
switch val := c.(type) {
case starlark.String:
s.whitelistedK8SContexts = append(s.whitelistedK8SContexts, k8s.KubeContext(val))
default:
return nil, fmt.Errorf("allow_k8s_contexts contexts must be a string or a sequence of strings; found a %T", val)

}
}

return starlark.None, nil
}

var ipv4Loopback = regexp.MustCompile(`^127(?:\.0){1,2}\.1$`)
var ipv6Loopback = "::1"

// this is non-exhaustive, but hopefully gets >99% of cases, without having to make a network request
// it doesn't cover ipv6-mapped ipv4 addresses (e.g., "http://[::ffff:7f00:1]"), non-loopback ips,
// or non-"localhost" hostnames mapped to loopback
func isLocalhost(host string) bool {
return host == "localhost" ||
ipv4Loopback.MatchString(host) ||
host == ipv6Loopback
}

func (s *tiltfileState) validateK8SContext(kubeConfig *api.Config) error {
contextName := kubeConfig.CurrentContext
clusterName := kubeConfig.Contexts[contextName].Cluster
server := kubeConfig.Clusters[clusterName].Server
url, err := url.Parse(server)
if err != nil {
return errors.Wrapf(err, "running in kube context %q, which specifies kube api url %q. error parsing that url", contextName, server)
}

if isLocalhost(url.Hostname()) {
return nil
}

for _, c := range s.whitelistedK8SContexts {
if c == k8s.KubeContext(contextName) {
return nil
}
}

return fmt.Errorf(
`Stop! '%s' might be production.
If you're sure you want to deploy there, add:
allow_k8s_contexts('%s')
to your Tiltfile. Otherwise, switch k8s contexts and restart Tilt.`,
contextName,
contextName)
}
9 changes: 9 additions & 0 deletions internal/tiltfile/tiltfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/pkg/errors"
"go.starlark.net/resolve"
"go.starlark.net/starlark"
"k8s.io/client-go/tools/clientcmd/api"

"github.com/windmilleng/tilt/internal/analytics"
"github.com/windmilleng/tilt/internal/dockercompose"
Expand Down Expand Up @@ -72,12 +73,14 @@ func ProvideTiltfileLoader(
kCli k8s.Client,
dcCli dockercompose.DockerComposeClient,
kubeContext k8s.KubeContext,
kubeConfig *api.Config,
fDefaults feature.Defaults) TiltfileLoader {
return tiltfileLoader{
analytics: analytics,
kCli: kCli,
dcCli: dcCli,
kubeContext: kubeContext,
kubeConfig: kubeConfig,
fDefaults: fDefaults,
}
}
Expand All @@ -87,6 +90,7 @@ type tiltfileLoader struct {
kCli k8s.Client
dcCli dockercompose.DockerComposeClient
kubeContext k8s.KubeContext
kubeConfig *api.Config
fDefaults feature.Defaults
}

Expand Down Expand Up @@ -141,6 +145,11 @@ func (tfl tiltfileLoader) Load(ctx context.Context, filename string, matching ma
if err != nil {
return TiltfileLoadResult{}, err
}

err = s.validateK8SContext(tfl.kubeConfig)
if err != nil {
return TiltfileLoadResult{}, err
}
} else {
manifests, err = s.translateDC(resources.dc)
if err != nil {
Expand Down
9 changes: 9 additions & 0 deletions internal/tiltfile/tiltfile_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ type tiltfileState struct {
k8sResourceAssemblyVersion int
k8sResourceAssemblyVersionReason k8sResourceAssemblyVersionReason
workloadToResourceFunction workloadToResourceFunction
whitelistedK8SContexts []k8s.KubeContext

// for assembly
usedImages map[string]bool
Expand Down Expand Up @@ -94,6 +95,11 @@ const (
k8sResourceAssemblyVersionReasonExplicit
)

// These are k8s context names that we assume are safe to deploy to even if they are neither localhost
// nor in allow_k8s_contexts. e.g., minikube uses a non-loopback ip on a virtual interface,
// and some versions of docker-for-desktop use 'kubernetes.docker.local'
var defaultWhitelistedKubeContexts = []k8s.KubeContext{"minikube", "docker-desktop", "docker-for-desktop"}
Copy link
Member

Choose a reason for hiding this comment

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

can this use the Env constants?

Copy link
Member Author

Choose a reason for hiding this comment

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

my main concern is having multiple codepaths that each invent their own definition of "local" cluster.

I'd forgotten about Env when writing this. Do you see any downside to just using Env.IsLocalCluster and skip checking the url?

I'm not very familiar with KIND, but it seems like the chief current use of Env.IsLocalCluster is to check whether can skip the push to docker, and we push to docker for KIND even though it's local? Which is to say, I'm not sure whether this method's goals align with the safety check's. Either way, though, it seems like it makes sense to put this logic in env.go.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, new take:

  1. renamed the existing Env.IsLocalCluster to Env.UsesLocalDockerRegistry, since that seems to better match its existing use, and then made a new Env.IsLocalCluster that also includes KinD.
  2. just use Env.IsLocalCluster and don't get clever in the tiltfile code


func newTiltfileState(ctx context.Context, dcCli dockercompose.DockerComposeClient, kubeContext k8s.KubeContext, privateRegistry container.Registry, features feature.FeatureSet) *tiltfileState {
return &tiltfileState{
ctx: ctx,
Expand All @@ -113,6 +119,7 @@ func newTiltfileState(ctx context.Context, dcCli dockercompose.DockerComposeClie
triggerMode: TriggerModeAuto,
features: features,
loadCache: make(map[string]loadCacheEntry),
whitelistedK8SContexts: defaultWhitelistedKubeContexts,
}
}

Expand Down Expand Up @@ -165,6 +172,7 @@ const (
k8sImageJSONPathN = "k8s_image_json_path"
workloadToResourceFunctionN = "workload_to_resource_function"
k8sContextN = "k8s_context"
allowK8SContexts = "allow_k8s_contexts"

// file functions
localGitRepoN = "local_git_repo"
Expand Down Expand Up @@ -294,6 +302,7 @@ func (s *tiltfileState) predeclared() starlark.StringDict {
addBuiltin(r, k8sImageJSONPathN, s.k8sImageJsonPath)
addBuiltin(r, workloadToResourceFunctionN, s.workloadToResourceFunctionFn)
addBuiltin(r, k8sContextN, s.k8sContext)
addBuiltin(r, allowK8SContexts, s.allowK8SContexts)
addBuiltin(r, localGitRepoN, s.localGitRepo)
addBuiltin(r, kustomizeN, s.kustomize)
addBuiltin(r, helmN, s.helm)
Expand Down
Loading