Change the tctl format for database_object_import_rule resource (#…

…39764)

* Fix incorrect expiry when using resource153ToLegacyAdapter

* Change the `tctl` format for database_object_import_rule resource.

Prior to this change the resource wouldn't round-trip through `tctl get ... | tctl create` because of header incompatibilities.

In general, all resources must be compatible with services.UnknownResource for their `tctl` storage format.

* Lint fix

lib/services/databaseobjectimportrule.go

@@ -21,12 +21,7 @@ package services
 import (
 	"context"
 
-	"github.com/gravitational/trace"
-	"google.golang.org/protobuf/proto"
-	"google.golang.org/protobuf/types/known/timestamppb"
-
 	dbobjectimportrulev1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/dbobjectimportrule/v1"
-	"github.com/gravitational/teleport/lib/utils"
 )
 
 // DatabaseObjectImportRules manages DatabaseObjectImportRule resources.
@@ -49,49 +44,3 @@ type DatabaseObjectImportRules interface {
 	// ListDatabaseObjectImportRules will list DatabaseObjectImportRule resources.
 	ListDatabaseObjectImportRules(ctx context.Context, size int, pageToken string) ([]*dbobjectimportrulev1.DatabaseObjectImportRule, string, error)
 }
-
-// MarshalDatabaseObjectImportRule marshals DatabaseObjectImportRule resource to JSON.
-func MarshalDatabaseObjectImportRule(rule *dbobjectimportrulev1.DatabaseObjectImportRule, opts ...MarshalOption) ([]byte, error) {
-	cfg, err := CollectOptions(opts)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-	if !cfg.PreserveResourceID {
-		rule = proto.Clone(rule).(*dbobjectimportrulev1.DatabaseObjectImportRule)
-		//nolint:staticcheck // SA1019. Deprecated, but still needed.
-		rule.Metadata.Id = 0
-		rule.Metadata.Revision = ""
-	}
-	data, err := utils.FastMarshal(rule)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-	return data, nil
-}
-
-// UnmarshalDatabaseObjectImportRule unmarshals the DatabaseObjectImportRule resource from JSON.
-func UnmarshalDatabaseObjectImportRule(data []byte, opts ...MarshalOption) (*dbobjectimportrulev1.DatabaseObjectImportRule, error) {
-	if len(data) == 0 {
-		return nil, trace.BadParameter("missing DatabaseObjectImportRule data")
-	}
-	cfg, err := CollectOptions(opts)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-	var obj dbobjectimportrulev1.DatabaseObjectImportRule
-	err = utils.FastUnmarshal(data, &obj)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-	if cfg.ID != 0 {
-		//nolint:staticcheck // SA1019. Id is deprecated, but still needed.
-		obj.Metadata.Id = cfg.ID
-	}
-	if cfg.Revision != "" {
-		obj.Metadata.Revision = cfg.Revision
-	}
-	if !cfg.Expires.IsZero() {
-		obj.Metadata.Expires = timestamppb.New(cfg.Expires)
-	}
-	return &obj, nil
-}

lib/services/local/databaseobjectimportrule.go

@@ -22,12 +22,15 @@ import (
 	"context"
 
 	"github.com/gravitational/trace"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/timestamppb"
 
 	databaseobjectimportrulev1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/dbobjectimportrule/v1"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/backend"
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/services/local/generic"
+	"github.com/gravitational/teleport/lib/utils"
 )
 
 // databaseObjectImportRuleService manages database object import rules in the backend.
@@ -74,10 +77,54 @@ func NewDatabaseObjectImportRuleService(backend backend.Backend) (services.Datab
 	service, err := generic.NewServiceWrapper(backend,
 		types.KindDatabaseObjectImportRule,
 		databaseObjectImportRulePrefix,
-		services.MarshalDatabaseObjectImportRule,
-		services.UnmarshalDatabaseObjectImportRule)
+		marshalDatabaseObjectImportRule,
+		unmarshalDatabaseObjectImportRule)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
 	return &databaseObjectImportRuleService{service: service}, nil
 }
+
+func marshalDatabaseObjectImportRule(rule *databaseobjectimportrulev1.DatabaseObjectImportRule, opts ...services.MarshalOption) ([]byte, error) {
+	cfg, err := services.CollectOptions(opts)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	if !cfg.PreserveResourceID {
+		rule = proto.Clone(rule).(*databaseobjectimportrulev1.DatabaseObjectImportRule)
+		//nolint:staticcheck // SA1019. Deprecated, but still needed.
+		rule.Metadata.Id = 0
+		rule.Metadata.Revision = ""
+	}
+	data, err := utils.FastMarshal(rule)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	return data, nil
+}
+
+func unmarshalDatabaseObjectImportRule(data []byte, opts ...services.MarshalOption) (*databaseobjectimportrulev1.DatabaseObjectImportRule, error) {
+	if len(data) == 0 {
+		return nil, trace.BadParameter("missing DatabaseObjectImportRule data")
+	}
+	cfg, err := services.CollectOptions(opts)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	var obj databaseobjectimportrulev1.DatabaseObjectImportRule
+	err = utils.FastUnmarshal(data, &obj)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	if cfg.ID != 0 {
+		//nolint:staticcheck // SA1019. Id is deprecated, but still needed.
+		obj.Metadata.Id = cfg.ID
+	}
+	if cfg.Revision != "" {
+		obj.Metadata.Revision = cfg.Revision
+	}
+	if !cfg.Expires.IsZero() {
+		obj.Metadata.Expires = timestamppb.New(cfg.Expires)
+	}
+	return &obj, nil
+}

lib/services/local/databaseobjectimportrule_test.go

@@ -29,8 +29,12 @@ import (
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
+	"github.com/gravitational/teleport/api/defaults"
 	databaseobjectimportrulev1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/dbobjectimportrule/v1"
+	headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
+	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/api/types/label"
+	apilabels "github.com/gravitational/teleport/api/types/label"
 	"github.com/gravitational/teleport/lib/backend/memory"
 	"github.com/gravitational/teleport/lib/srv/db/common/databaseobjectimportrule"
 )
@@ -170,3 +174,42 @@ func TestDatabaseObjectImportRuleCRUD(t *testing.T) {
 	require.Empty(t, nextToken)
 	require.Empty(t, out)
 }
+
+func TestMarshalDatabaseObjectImportRuleRoundTrip(t *testing.T) {
+	spec := &databaseobjectimportrulev1.DatabaseObjectImportRuleSpec{
+		Priority:       30,
+		DatabaseLabels: apilabels.FromMap(map[string][]string{"env": {"staging", "prod"}, "owner_org": {"trading"}}),
+		Mappings: []*databaseobjectimportrulev1.DatabaseObjectImportRuleMapping{
+			{
+				Scope: &databaseobjectimportrulev1.DatabaseObjectImportScope{
+					SchemaNames:   []string{"public"},
+					DatabaseNames: []string{"foo", "bar", "baz"},
+				},
+				Match: &databaseobjectimportrulev1.DatabaseObjectImportMatch{
+					TableNames:     []string{"*"},
+					ViewNames:      []string{"1", "2", "3"},
+					ProcedureNames: []string{"aaa", "bbb", "ccc"},
+				},
+				AddLabels: map[string]string{
+					"env":          "staging",
+					"custom_label": "my_custom_value",
+				},
+			},
+		},
+	}
+	obj := &databaseobjectimportrulev1.DatabaseObjectImportRule{
+		Kind:    types.KindDatabaseObjectImportRule,
+		Version: types.V1,
+		Metadata: &headerv1.Metadata{
+			Name:      "import_all_staging_tables",
+			Namespace: defaults.Namespace,
+		},
+		Spec: spec,
+	}
+
+	out, err := marshalDatabaseObjectImportRule(obj)
+	require.NoError(t, err)
+	newObj, err := unmarshalDatabaseObjectImportRule(out)
+	require.NoError(t, err)
+	require.True(t, proto.Equal(obj, newObj), "messages are not equal")
+}

lib/services/resource.go

@@ -654,13 +654,6 @@ func init() {
 		}
 		return types.Resource153ToLegacy(b), nil
 	})
-	RegisterResourceUnmarshaler(types.KindDatabaseObjectImportRule, func(bytes []byte, opts ...MarshalOption) (types.Resource, error) {
-		out, err := UnmarshalDatabaseObjectImportRule(bytes, opts...)
-		if err != nil {
-			return nil, trace.Wrap(err)
-		}
-		return types.Resource153ToLegacy(out), nil
-	})
 }
 
 // CheckAndSetDefaults calls [r.CheckAndSetDefaults] if r implements the method.

tool/tctl/common/collection.go

@@ -47,6 +47,7 @@ import (
 	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/teleport/tool/common"
 	"github.com/gravitational/teleport/tool/tctl/common/databaseobject"
+	"github.com/gravitational/teleport/tool/tctl/common/databaseobjectimportrule"
 	"github.com/gravitational/teleport/tool/tctl/common/loginrule"
 	"github.com/gravitational/teleport/tool/tctl/common/oktaassignment"
 )
@@ -1174,7 +1175,7 @@ type databaseObjectImportRuleCollection struct {
 func (c *databaseObjectImportRuleCollection) resources() []types.Resource {
 	resources := make([]types.Resource, len(c.rules))
 	for i, b := range c.rules {
-		resources[i] = types.Resource153ToLegacy(b)
+		resources[i] = databaseobjectimportrule.ProtoToResource(b)
 	}
 	return resources
 }

tool/tctl/common/databaseobjectimportrule/resource.go

@@ -0,0 +1,87 @@
+// Teleport
+// Copyright (C) 2024 Gravitational, Inc.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package databaseobjectimportrule
+
+import (
+	"github.com/gravitational/trace"
+	"google.golang.org/protobuf/types/known/timestamppb"
+
+	"github.com/gravitational/teleport/api/defaults"
+	dbobjectimportrulev1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/dbobjectimportrule/v1"
+	headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1"
+	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/lib/utils"
+)
+
+// Resource is a type wrapper type for YAML (un)marshaling.
+type Resource struct {
+	// ResourceHeader is embedded to implement types.Resource
+	types.ResourceHeader
+	// Spec is the database object specification
+	Spec *dbobjectimportrulev1.DatabaseObjectImportRuleSpec `json:"spec"`
+}
+
+// UnmarshalJSON parses Resource and converts into an object.
+func UnmarshalJSON(raw []byte) (*dbobjectimportrulev1.DatabaseObjectImportRule, error) {
+	var resource Resource
+	if err := utils.FastUnmarshal(raw, &resource); err != nil {
+		return nil, trace.Wrap(err)
+	}
+	return ResourceToProto(&resource), nil
+}
+
+// ProtoToResource converts a *dbobjectimportrulev1.DatabaseObjectImportRule into a *Resource which
+// implements types.Resource and can be marshaled to YAML or JSON in a
+// human-friendly format.
+func ProtoToResource(rule *dbobjectimportrulev1.DatabaseObjectImportRule) *Resource {
+	r := &Resource{
+		ResourceHeader: types.ResourceHeader{
+			Kind:     rule.GetKind(),
+			SubKind:  rule.GetSubKind(),
+			Version:  rule.GetVersion(),
+			Metadata: types.Resource153ToLegacy(rule).GetMetadata(),
+		},
+		Spec: rule.GetSpec(),
+	}
+	return r
+}
+
+func ResourceToProto(r *Resource) *dbobjectimportrulev1.DatabaseObjectImportRule {
+	md := r.Metadata
+
+	var expires *timestamppb.Timestamp
+	if md.Expires != nil {
+		expires = timestamppb.New(*md.Expires)
+	}
+
+	return &dbobjectimportrulev1.DatabaseObjectImportRule{
+		Kind:    r.GetKind(),
+		SubKind: r.GetSubKind(),
+		Version: r.GetVersion(),
+		Metadata: &headerv1.Metadata{
+			Name:        md.Name,
+			Description: md.Description,
+			Namespace:   defaults.Namespace,
+			Labels:      md.Labels,
+			Expires:     expires,
+			//nolint:staticcheck // SA1019. Id is deprecated.
+			Id:       md.ID,
+			Revision: md.Revision,
+		},
+		Spec: r.Spec,
+	}
+}

tool/tctl/common/resource_command.go

@@ -63,6 +63,7 @@ import (
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/teleport/tool/tctl/common/databaseobject"
+	"github.com/gravitational/teleport/tool/tctl/common/databaseobjectimportrule"
 	"github.com/gravitational/teleport/tool/tctl/common/loginrule"
 )
 
@@ -596,7 +597,7 @@ func (rc *ResourceCommand) createBot(ctx context.Context, client *auth.Client, r
 }
 
 func (rc *ResourceCommand) createDatabaseObjectImportRule(ctx context.Context, client *auth.Client, raw services.UnknownResource) error {
-	rule, err := services.UnmarshalDatabaseObjectImportRule(raw.Raw)
+	rule, err := databaseobjectimportrule.UnmarshalJSON(raw.Raw)
 	if err != nil {
 		return trace.Wrap(err)
 	}