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

Use managed resource name as external name #45

Merged
merged 5 commits into from
Oct 15, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 27 additions & 0 deletions pkg/meta/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ import (
satisfy metav1.Object.
*/

const (
// ExternalNameAnnotationKey is the key in the annotations map of a resource
// for the name of the resource as it appears on provider's systems.
ExternalNameAnnotationKey = "crossplane.io/external-name"
)

// ReferenceTo returns an object reference to the supplied object, presumed to
// be of the supplied group, version, and kind.
func ReferenceTo(o metav1.Object, of schema.GroupVersionKind) *corev1.ObjectReference {
Expand Down Expand Up @@ -131,6 +137,17 @@ func RemoveFinalizer(o metav1.Object, finalizer string) {
o.SetFinalizers(f)
}

// FinalizerExists checks whether given finalizer is already set.
func FinalizerExists(o metav1.Object, finalizer string) bool {
f := o.GetFinalizers()
for _, e := range f {
if e == finalizer {
return true
}
}
return false
}

// AddLabels to the supplied object.
func AddLabels(o metav1.Object, labels map[string]string) {
l := o.GetLabels()
Expand Down Expand Up @@ -193,3 +210,13 @@ func WasCreated(o metav1.Object) bool {
t := o.GetCreationTimestamp()
return !t.IsZero()
}

// GetExternalName returns the external name annotation value on the resource.
func GetExternalName(o metav1.Object) string {
return o.GetAnnotations()[ExternalNameAnnotationKey]
}

// SetExternalName sets the external name annotation of the resource.
func SetExternalName(o metav1.Object, name string) {
AddAnnotations(o, map[string]string{ExternalNameAnnotationKey: name})
}
105 changes: 105 additions & 0 deletions pkg/meta/meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,59 @@ func TestRemoveFinalizer(t *testing.T) {
}
}

func TestFinalizerExists(t *testing.T) {
finalizer := "fin"
funalizer := "fun"
Copy link
Member

Choose a reason for hiding this comment

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

I approve of this test value. 👍


type args struct {
o metav1.Object
finalizer string
}

cases := map[string]struct {
args args
want bool
}{
"NoExistingFinalizers": {
args: args{
o: &corev1.Pod{},
finalizer: finalizer,
},
want: false,
},
"FinalizerExists": {
args: args{
o: &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Finalizers: []string{finalizer},
},
},
finalizer: finalizer,
},
want: true,
},
"AnotherFinalizerExists": {
args: args{
o: &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Finalizers: []string{funalizer},
},
},
finalizer: finalizer,
},
want: false,
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
if diff := cmp.Diff(tc.want, FinalizerExists(tc.args.o, tc.args.finalizer)); diff != "" {
t.Errorf("tc.args.o.GetFinalizers(...): -want, +got:\n%s", diff)
}
})
}
}

func TestAddLabels(t *testing.T) {
key, value := "key", "value"
existingKey, existingValue := "ekey", "evalue"
Expand Down Expand Up @@ -755,3 +808,55 @@ func TestWasCreated(t *testing.T) {
})
}
}

func TestGetExternalName(t *testing.T) {
testName := "my-external-name"

cases := map[string]struct {
o metav1.Object
want string
}{
"ExternalNameExists": {
o: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ExternalNameAnnotationKey: testName}}},
want: testName,
},
"NoExternalName": {
o: &corev1.Pod{},
want: "",
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
got := GetExternalName(tc.o)
if diff := cmp.Diff(tc.want, got); diff != "" {
t.Errorf("WasCreated(...): -want, +got:\n%s", diff)
}
})
}
}

func TestSetExternalName(t *testing.T) {
testName := "my-external-name"

cases := map[string]struct {
o metav1.Object
name string
want metav1.Object
}{
"SetsTheCorrectKey": {
o: &corev1.Pod{},
name: testName,
want: &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ExternalNameAnnotationKey: testName}}},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
SetExternalName(tc.o, tc.name)
if diff := cmp.Diff(tc.want, tc.o); diff != "" {
t.Errorf("WasCreated(...): -want, +got:\n%s", diff)
}
})
}
}
54 changes: 52 additions & 2 deletions pkg/resource/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ func NewAPIManagedBinder(c client.Client) *APIManagedBinder {
// Bind the supplied resource to the supplied claim.
func (a *APIManagedBinder) Bind(ctx context.Context, cm Claim, mg Managed) error {
cm.SetBindingPhase(v1alpha1.BindingPhaseBound)
// Propagate back the final name of the external resource to the claim.
if meta.GetExternalName(mg) != "" {
negz marked this conversation as resolved.
Show resolved Hide resolved
meta.SetExternalName(cm, meta.GetExternalName(mg))
muvaf marked this conversation as resolved.
Show resolved Hide resolved
if err := a.client.Update(ctx, cm); err != nil {
return errors.Wrap(err, errUpdateClaim)
}
}
mg.SetBindingPhase(v1alpha1.BindingPhaseBound)
if err := a.client.Update(ctx, mg); err != nil {
return errors.Wrap(err, errUpdateManaged)
Expand All @@ -177,6 +184,13 @@ func NewAPIManagedStatusBinder(c client.Client) *APIManagedStatusBinder {
// Bind the supplied resource to the supplied claim.
func (a *APIManagedStatusBinder) Bind(ctx context.Context, cm Claim, mg Managed) error {
cm.SetBindingPhase(v1alpha1.BindingPhaseBound)
// Propagate back the final name of the external resource to the claim.
if meta.GetExternalName(mg) != "" {
meta.SetExternalName(cm, meta.GetExternalName(mg))
if err := a.client.Update(ctx, cm); err != nil {
return errors.Wrap(err, errUpdateClaim)
}
}
mg.SetBindingPhase(v1alpha1.BindingPhaseBound)
if err := a.client.Status().Update(ctx, mg); err != nil {
return errors.Wrap(err, errUpdateManagedStatus)
Expand Down Expand Up @@ -263,6 +277,20 @@ func (a *APIManagedFinalizerRemover) Finalize(ctx context.Context, mg Managed) e
return errors.Wrap(a.client.Update(ctx, mg), errUpdateManaged)
}

// A InitializerChain chains multiple managed initializers.
type InitializerChain []ManagedInitializer

// Initialize calls each ManagedInitializer serially. It returns the first
// error it encounters, if any.
func (cc InitializerChain) Initialize(ctx context.Context, mg Managed) error {
for _, c := range cc {
if err := c.Initialize(ctx, mg); err != nil {
return err
}
}
return nil
}

// An APIManagedFinalizerAdder establishes ownership of a managed resource by
// adding a finalizer and updating it in the API server.
type APIManagedFinalizerAdder struct{ client client.Client }
Expand All @@ -272,8 +300,30 @@ func NewAPIManagedFinalizerAdder(c client.Client) *APIManagedFinalizerAdder {
return &APIManagedFinalizerAdder{client: c}
}

// Establish ownership of the supplied Managed resource.
func (a *APIManagedFinalizerAdder) Establish(ctx context.Context, mg Managed) error {
// Initialize ownership of the supplied Managed resource.
func (a *APIManagedFinalizerAdder) Initialize(ctx context.Context, mg Managed) error {
if meta.FinalizerExists(mg, managedFinalizerName) {
return nil
}
meta.AddFinalizer(mg, managedFinalizerName)
return errors.Wrap(a.client.Update(ctx, mg), errUpdateManaged)
}

// ManagedNameAsExternalName writes the name of the managed resource to
// the external name annotation field in order to be used as name of
// the external resource in provider.
type ManagedNameAsExternalName struct{ client client.Client }

// NewManagedNameAsExternalName returns a new ManagedNameAsExternalName.
func NewManagedNameAsExternalName(c client.Client) *ManagedNameAsExternalName {
return &ManagedNameAsExternalName{client: c}
}

// Initialize the given managed resource.
func (a *ManagedNameAsExternalName) Initialize(ctx context.Context, mg Managed) error {
if meta.GetExternalName(mg) != "" {
return nil
}
meta.SetExternalName(mg, mg.GetName())
return errors.Wrap(a.client.Update(ctx, mg), errUpdateManaged)
}
90 changes: 85 additions & 5 deletions pkg/resource/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ var (
_ ManagedBinder = &APIManagedBinder{}
_ ManagedBinder = &APIManagedStatusBinder{}
_ ClaimFinalizer = &APIClaimFinalizerRemover{}
_ ManagedEstablisher = &APIManagedFinalizerAdder{}
_ ManagedInitializer = &APIManagedFinalizerAdder{}
_ ManagedInitializer = &ManagedNameAsExternalName{}
_ ManagedFinalizer = &APIManagedFinalizerRemover{}
)

Expand Down Expand Up @@ -836,7 +837,7 @@ func TestFinalizeManaged(t *testing.T) {
}
}

func TestEstablishManaged(t *testing.T) {
func TestAPIManagedFinalizerAdder(t *testing.T) {
type args struct {
ctx context.Context
mg Managed
Expand Down Expand Up @@ -881,12 +882,91 @@ func TestEstablishManaged(t *testing.T) {
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
api := NewAPIManagedFinalizerAdder(tc.client)
err := api.Establish(tc.args.ctx, tc.args.mg)
err := api.Initialize(tc.args.ctx, tc.args.mg)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("api.Establish(...): -want error, +got error:\n%s", diff)
t.Errorf("api.Initialize(...): -want error, +got error:\n%s", diff)
}
if diff := cmp.Diff(tc.want.mg, tc.args.mg, test.EquateConditions()); diff != "" {
t.Errorf("api.Establish(...) Managed: -want, +got:\n%s", diff)
t.Errorf("api.Initialize(...) Managed: -want, +got:\n%s", diff)
}
})
}
}

func TestManagedNameAsExternalName(t *testing.T) {
type args struct {
ctx context.Context
mg Managed
}

type want struct {
err error
mg Managed
}

errBoom := errors.New("boom")
testExternalName := "my-external-name"

cases := map[string]struct {
client client.Client
args args
want want
}{
"UpdateManagedError": {
client: &test.MockClient{MockUpdate: test.NewMockUpdateFn(errBoom)},
args: args{
ctx: context.Background(),
mg: &MockManaged{ObjectMeta: metav1.ObjectMeta{Name: testExternalName}},
},
want: want{
err: errors.Wrap(errBoom, errUpdateManaged),
mg: &MockManaged{ObjectMeta: metav1.ObjectMeta{
Name: testExternalName,
Annotations: map[string]string{meta.ExternalNameAnnotationKey: testExternalName},
}},
},
},
"UpdateSuccessful": {
client: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
args: args{
ctx: context.Background(),
mg: &MockManaged{ObjectMeta: metav1.ObjectMeta{Name: testExternalName}},
},
want: want{
err: nil,
mg: &MockManaged{ObjectMeta: metav1.ObjectMeta{
Name: testExternalName,
Annotations: map[string]string{meta.ExternalNameAnnotationKey: testExternalName},
}},
},
},
"UpdateNotNeeded": {
args: args{
ctx: context.Background(),
mg: &MockManaged{ObjectMeta: metav1.ObjectMeta{
Name: testExternalName,
Annotations: map[string]string{meta.ExternalNameAnnotationKey: "some-name"},
}},
},
want: want{
err: nil,
mg: &MockManaged{ObjectMeta: metav1.ObjectMeta{
Name: testExternalName,
Annotations: map[string]string{meta.ExternalNameAnnotationKey: "some-name"},
}},
},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
api := NewManagedNameAsExternalName(tc.client)
err := api.Initialize(tc.args.ctx, tc.args.mg)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("api.Initialize(...): -want error, +got error:\n%s", diff)
}
if diff := cmp.Diff(tc.want.mg, tc.args.mg, test.EquateConditions()); diff != "" {
t.Errorf("api.Initialize(...) Managed: -want, +got:\n%s", diff)
}
})
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/resource/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ func NewObjectMetaConfigurator(t runtime.ObjectTyper) *ObjectMetaConfigurator {
func (c *ObjectMetaConfigurator) Configure(_ context.Context, cm Claim, cs NonPortableClass, mg Managed) error {
mg.SetNamespace(cs.GetNamespace())
mg.SetGenerateName(fmt.Sprintf("%s-%s-", cm.GetNamespace(), cm.GetName()))

if meta.GetExternalName(cm) != "" {
meta.SetExternalName(mg, meta.GetExternalName(cm))
}
// TODO(negz): Don't set this potentially cross-namespace owner reference.
// We probably want to use the resource's reclaim policy, not Kubernetes
// garbage collection, to determine whether to delete a managed resource
Expand Down
Loading