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

Implement CheckConfig/DiffConfig #577

Merged
merged 4 commits into from
Jun 4, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
### Improvements

- Enable configuring `ResourceOptions` via `transformations` (https://github.com/pulumi/pulumi-kubernetes/pull/575).
- Changing k8s cluster config now correctly causes dependent resources to be replaced (https://github.com/pulumi/pulumi-kubernetes/pull/577).

### Bug fixes

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/google/gofuzz v1.0.0 // indirect
github.com/googleapis/gnostic v0.2.0
github.com/gophercloud/gophercloud v0.0.0-20190418141522-bb98932a7b3a // indirect
github.com/grpc/grpc-go v0.0.0-00010101000000-000000000000
github.com/grpc/grpc-go v0.0.0-00010101000000-000000000000 // indirect
github.com/imdario/mergo v0.3.7 // indirect
github.com/jinzhu/copier v0.0.0-20180308034124-7e38e58719c3
github.com/json-iterator/go v1.1.6 // indirect
Expand Down
88 changes: 85 additions & 3 deletions pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ import (
"fmt"
"net/url"
"os"
"reflect"
"strings"

"github.com/golang/glog"
pbempty "github.com/golang/protobuf/ptypes/empty"
structpb "github.com/golang/protobuf/ptypes/struct"
"github.com/grpc/grpc-go/status"
pkgerrors "github.com/pkg/errors"
"github.com/pulumi/pulumi-kubernetes/pkg/await"
"github.com/pulumi/pulumi-kubernetes/pkg/clients"
Expand Down Expand Up @@ -109,12 +109,94 @@ func makeKubeProvider(

// CheckConfig validates the configuration for this provider.
func (k *kubeProvider) CheckConfig(ctx context.Context, req *pulumirpc.CheckRequest) (*pulumirpc.CheckResponse, error) {
return nil, status.Error(codes.Unimplemented, "CheckConfig is not yet implemented")
return &pulumirpc.CheckResponse{Inputs: req.GetNews()}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder, do we need to start returning "defaults" here? For example, if you are using the default provider and not explicitly setting a cluster or context or kubeconfig in your stack's configuration, I think this is going to return an empty set of inputs. If later, you go and change the kubeconfig by hand and re-run up, we won't see any changes. Is that actually the behavior we want here?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we do that, we should be careful about cases where in DiffConfig the "olds" are empty, since when upgrading to a newer CLI and a provider with this change, the initial call to DiffConfig will more or less always pass in an empty olds bag.

Copy link
Member

Choose a reason for hiding this comment

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

If later, you go and change the kubeconfig by hand and re-run up, we won't see any changes. Is that actually the behavior we want here?

FWIW, that behavior may be consistent with what we will get out of the TF providers by default. Those providers may end up automagically picking up some optional settings by way of the cloud provider SDKs (e.g. I think the AWS Go SDK will pick up certain things that are not reflected in the provider config; @jen20 may be able to confirm).

Copy link
Contributor

Choose a reason for hiding this comment

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

(e.g. I think the AWS Go SDK will pick up certain things that are not reflected in the provider config; @jen20 may be able to confirm).

Right, I agree that what we have here is similar to the default behavior for TF based providers, my question was more about the UX we /want/ to have.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the config is a combination of explicitly set values and what we pick up from awsbase.Credentials and our own provider preconfigure call back.

}

// DiffConfig diffs the configuration for this provider.
func (k *kubeProvider) DiffConfig(ctx context.Context, req *pulumirpc.DiffRequest) (*pulumirpc.DiffResponse, error) {
return nil, status.Error(codes.Unimplemented, "DiffConfig is not yet implemented")
urn := resource.URN(req.GetUrn())
lblackstone marked this conversation as resolved.
Show resolved Hide resolved
label := fmt.Sprintf("%s.DiffConfig(%s)", k.label(), urn)
glog.V(9).Infof("%s executing", label)

olds, err := plugin.UnmarshalProperties(req.GetOlds(), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.olds", label),
KeepUnknowns: true,
SkipNulls: true,
})
if err != nil {
return nil, err
}
news, err := plugin.UnmarshalProperties(req.GetNews(), plugin.MarshalOptions{
Label: fmt.Sprintf("%s.news", label),
KeepUnknowns: true,
SkipNulls: true,
})
if err != nil {
return nil, err
}

// We can't tell for sure if a computed value has changed, so we make the conservative choice
// and force a replacement.
if news["kubeconfig"].IsComputed() {
return &pulumirpc.DiffResponse{
Changes: pulumirpc.DiffResponse_DIFF_SOME,
Diffs: []string{"kubeconfig"},
Replaces: []string{"kubeconfig"},
}, nil
}

var diffs, replaces []string

oldConfig, err := parseKubeconfigPropertyValue(olds["kubeconfig"])
if err != nil {
return nil, err
}
newConfig, err := parseKubeconfigPropertyValue(news["kubeconfig"])
if err != nil {
return nil, err
}

// Check for differences in provider overrides.
if !reflect.DeepEqual(oldConfig, newConfig) {
diffs = append(diffs, "kubeconfig")
}
if olds["context"] != news["context"] {
diffs = append(diffs, "context")
}
if olds["cluster"] != news["cluster"] {
diffs = append(diffs, "cluster")
}

// In general, it's not possible to tell from a kubeconfig if the k8s cluster it points to has
// changed. k8s clusters do not have a well defined identity, so the best we can do is check
// if the settings for the active cluster have changed. This is not a foolproof method; a trivial
// counterexample is changing the load balancer or DNS entry pointing to the same cluster.
//
// Given this limitation, we try to strike a reasonable balance by planning a replacement iff
// the active cluster in the kubeconfig changes. This could still plan an erroneous replacement,
// but should work for the majority of cases.
//
// The alternative of ignoring changes to the kubeconfig is untenable; if the k8s cluster has
// changed, any dependent resources must be recreated, and ignoring changes prevents that from
// happening.
oldActiveCluster := getActiveClusterFromConfig(oldConfig, olds)
activeCluster := getActiveClusterFromConfig(newConfig, news)
if !reflect.DeepEqual(oldActiveCluster, activeCluster) {
replaces = diffs
}
glog.V(7).Infof("%s: diffs %v / replaces %v\n", label, diffs, replaces)
lblackstone marked this conversation as resolved.
Show resolved Hide resolved

if len(diffs) > 0 || len(replaces) > 0 {
return &pulumirpc.DiffResponse{
Changes: pulumirpc.DiffResponse_DIFF_SOME,
Diffs: diffs,
Replaces: replaces,
}, nil
}

return &pulumirpc.DiffResponse{
Changes: pulumirpc.DiffResponse_DIFF_NONE,
}, nil
}

// Configure configures the resource provider with "globals" that control its behavior.
Expand Down
43 changes: 43 additions & 0 deletions pkg/provider/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"github.com/pulumi/pulumi/pkg/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/tools/clientcmd"
clientapi "k8s.io/client-go/tools/clientcmd/api"
)

func hasComputedValue(obj *unstructured.Unstructured) bool {
Expand Down Expand Up @@ -61,3 +63,44 @@ func FqName(namespace, name string) string {
}
return fmt.Sprintf("%s/%s", namespace, name)
}

// --------------------------------------------------------------------------
// Kubeconfig helpers.
// --------------------------------------------------------------------------

// parseKubeconfigPropertyValue takes a PropertyValue that possibly contains a raw kubeconfig
// (YAML or JSON) string and attempts to unmarshal it into a Config struct. If the property value
// is empty, an empty Config is returned.
func parseKubeconfigPropertyValue(kubeconfig resource.PropertyValue) (*clientapi.Config, error) {
if kubeconfig.IsNull() {
return &clientapi.Config{}, nil
}

config, err := clientcmd.Load([]byte(kubeconfig.StringValue()))
if err != nil {
return nil, fmt.Errorf("failed to parse kubeconfig: %v", err)
}

return config, nil
}

// getActiveClusterFromConfig gets the current cluster from a kubeconfig, accounting for provider overrides.
func getActiveClusterFromConfig(config *clientapi.Config, overrides resource.PropertyMap) *clientapi.Cluster {
if len(config.Clusters) == 0 {
return &clientapi.Cluster{}
}

currentContext := config.CurrentContext
if val := overrides["context"]; !val.IsNull() {
currentContext = val.StringValue()
}

activeClusterName := config.Contexts[currentContext].Cluster

activeCluster := config.Clusters[activeClusterName]
if val := overrides["cluster"]; !val.IsNull() {
activeCluster = config.Clusters[val.StringValue()]
}

return activeCluster
}