diff --git a/CHANGELOG.md b/CHANGELOG.md index e739acb2..540d6dd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- [#184](https://github.com/deviceinsight/kafkactl/pull/184) Added option to show default configs when describing topics + ## 3.5.1 - 2023-11-10 ## 3.5.0 - 2023-11-10 diff --git a/cmd/describe/describe-topic.go b/cmd/describe/describe-topic.go index fa6f1db9..c95007e1 100644 --- a/cmd/describe/describe-topic.go +++ b/cmd/describe/describe-topic.go @@ -26,7 +26,7 @@ func newDescribeTopicCmd() *cobra.Command { } cmdDescribeTopic.Flags().StringVarP(&flags.OutputFormat, "output", "o", flags.OutputFormat, "output format. One of: json|yaml|wide") - cmdDescribeTopic.Flags().BoolVarP(&flags.PrintConfigs, "print-configs", "c", true, "print configs") + cmdDescribeTopic.Flags().StringVarP((*string)(&flags.PrintConfigs), "print-configs", "c", "no_defaults", "print configs. One of none|no_defaults|all") cmdDescribeTopic.Flags().BoolVarP(&flags.SkipEmptyPartitions, "skip-empty", "s", false, "show only partitions that have a messages") return cmdDescribeTopic diff --git a/cmd/describe/describe-topic_test.go b/cmd/describe/describe-topic_test.go index 285e0854..bcba19b3 100644 --- a/cmd/describe/describe-topic_test.go +++ b/cmd/describe/describe-topic_test.go @@ -4,9 +4,69 @@ import ( "strings" "testing" + "github.com/deviceinsight/kafkactl/internal" + "github.com/deviceinsight/kafkactl/internal/topic" + "github.com/deviceinsight/kafkactl/testutil" ) +func TestDescribeTopicConfigsIntegration(t *testing.T) { + + testutil.StartIntegrationTest(t) + + prefix := "describe-t-configs-" + + topicName1 := testutil.CreateTopic(t, prefix, "--config", "retention.ms=3600000") + + kafkaCtl := testutil.CreateKafkaCtlCommand() + kafkaCtl.Verbose = false + + // default --print-configs=no_defaults + if _, err := kafkaCtl.Execute("describe", "topic", topicName1, "-o", "yaml"); err != nil { + t.Fatalf("failed to execute command: %v", err) + } + + describedTopic, err := topic.FromYaml(kafkaCtl.GetStdOut()) + if err != nil { + t.Fatalf("failed to read yaml: %v", err) + } + + configKeys := getConfigKeys(describedTopic.Configs) + + testutil.AssertArraysEquals(t, []string{"retention.ms"}, configKeys) + testutil.AssertEquals(t, "3600000", describedTopic.Configs[0].Value) + + // explicitly without defaults + if _, err := kafkaCtl.Execute("describe", "topic", topicName1, "-c", "no_defaults", "-o", "yaml"); err != nil { + t.Fatalf("failed to execute command: %v", err) + } + + describedTopic, err = topic.FromYaml(kafkaCtl.GetStdOut()) + if err != nil { + t.Fatalf("failed to read yaml: %v", err) + } + + configKeys = getConfigKeys(describedTopic.Configs) + + testutil.AssertArraysEquals(t, []string{"retention.ms"}, configKeys) + testutil.AssertEquals(t, "3600000", describedTopic.Configs[0].Value) + + // all configs + if _, err := kafkaCtl.Execute("describe", "topic", topicName1, "-c", "all", "-o", "yaml"); err != nil { + t.Fatalf("failed to execute command: %v", err) + } + + describedTopic, err = topic.FromYaml(kafkaCtl.GetStdOut()) + if err != nil { + t.Fatalf("failed to read yaml: %v", err) + } + + configKeys = getConfigKeys(describedTopic.Configs) + + testutil.AssertContains(t, "retention.ms", configKeys) + testutil.AssertContains(t, "cleanup.policy", configKeys) +} + func TestDescribeTopicAutoCompletionIntegration(t *testing.T) { testutil.StartIntegrationTest(t) @@ -30,3 +90,11 @@ func TestDescribeTopicAutoCompletionIntegration(t *testing.T) { testutil.AssertContains(t, topicName2, outputLines) testutil.AssertContains(t, topicName3, outputLines) } + +func getConfigKeys(configs []internal.Config) []string { + keys := make([]string, len(configs)) + for i, config := range configs { + keys[i] = config.Name + } + return keys +} diff --git a/internal/broker/broker-operation.go b/internal/broker/broker-operation.go index 34446963..711a1c82 100644 --- a/internal/broker/broker-operation.go +++ b/internal/broker/broker-operation.go @@ -77,7 +77,7 @@ func (operation *Operation) GetBrokers(flags GetBrokersFlags) error { Name: fmt.Sprint(broker.ID()), } - if configs, err = internal.ListConfigs(&admin, brokerConfig); err != nil { + if configs, err = internal.ListConfigs(&admin, brokerConfig, false); err != nil { return err } @@ -153,7 +153,7 @@ func (operation *Operation) DescribeBroker(id int32, flags DescribeBrokerFlags) Name: fmt.Sprint(broker.ID()), } - if configs, err = internal.ListConfigs(&admin, brokerConfig); err != nil { + if configs, err = internal.ListConfigs(&admin, brokerConfig, false); err != nil { return err } diff --git a/internal/common-operation.go b/internal/common-operation.go index 53044ec7..56c0ac4f 100644 --- a/internal/common-operation.go +++ b/internal/common-operation.go @@ -309,10 +309,9 @@ func TopicExists(client *sarama.Client, name string) (bool, error) { return false, nil } -func ListConfigs(admin *sarama.ClusterAdmin, resource sarama.ConfigResource) ([]Config, error) { +func ListConfigs(admin *sarama.ClusterAdmin, resource sarama.ConfigResource, includeDefaults bool) ([]Config, error) { var ( - configs = make([]Config, 0) configEntries []sarama.ConfigEntry err error ) @@ -321,15 +320,21 @@ func ListConfigs(admin *sarama.ClusterAdmin, resource sarama.ConfigResource) ([] return nil, errors.Wrap(err, fmt.Sprintf("failed to describe %v config", getResourceTypeName(resource.Type))) } + return listConfigsFromEntries(configEntries, includeDefaults), nil +} + +func listConfigsFromEntries(configEntries []sarama.ConfigEntry, includeDefaults bool) []Config { + var configs = make([]Config, 0) + for _, configEntry := range configEntries { - if !configEntry.Default && configEntry.Source != sarama.SourceDefault { + if includeDefaults || (!configEntry.Default && configEntry.Source != sarama.SourceDefault) { entry := Config{Name: configEntry.Name, Value: configEntry.Value} configs = append(configs, entry) } } - return configs, nil + return configs } func getResourceTypeName(resourceType sarama.ConfigResourceType) string { diff --git a/internal/common-operation_test.go b/internal/common-operation_test.go new file mode 100644 index 00000000..a4fc3ec7 --- /dev/null +++ b/internal/common-operation_test.go @@ -0,0 +1,83 @@ +package internal + +import ( + "reflect" + "testing" + + "github.com/IBM/sarama" +) + +func TestListConfigsFromEntries(t *testing.T) { + testCases := []struct { + name string + entries []sarama.ConfigEntry + includeDefaults bool + configs []Config + }{ + { + name: "not include defaults, empty entries", + entries: []sarama.ConfigEntry{}, + configs: []Config{}, + }, + { + name: "not include defaults", + entries: []sarama.ConfigEntry{ + { + Name: "non_default", + Value: "ND", + Default: false, + Source: sarama.SourceUnknown, + }, + { + Name: "default", + Value: "D", + Default: true, + Source: sarama.SourceDefault, + }, + }, + configs: []Config{ + {Name: "non_default", Value: "ND"}, + }, + }, + { + name: "include defaults, empty entries", + entries: []sarama.ConfigEntry{}, + configs: []Config{}, + includeDefaults: true, + }, + { + name: "include defaults", + entries: []sarama.ConfigEntry{ + { + Name: "non_default", + Value: "ND", + Default: false, + Source: sarama.SourceUnknown, + }, + { + Name: "default", + Value: "D", + Default: true, + Source: sarama.SourceDefault, + }, + }, + configs: []Config{ + {Name: "non_default", Value: "ND"}, + {Name: "default", Value: "D"}, + }, + includeDefaults: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + configs := listConfigsFromEntries(tc.entries, tc.includeDefaults) + + if len(configs) > 0 && + len(tc.configs) > 0 && + !reflect.DeepEqual(configs, tc.configs) { + t.Fatalf("expect: %v, got %v", tc.configs, configs) + } + }) + } +} diff --git a/internal/topic/topic-operation.go b/internal/topic/topic-operation.go index 3f019992..464174e5 100644 --- a/internal/topic/topic-operation.go +++ b/internal/topic/topic-operation.go @@ -37,10 +37,13 @@ type requestedTopicFields struct { partitionLeader bool partitionReplicas bool partitionISRs bool - config bool + config PrintConfigsParam } -var allFields = requestedTopicFields{partitionID: true, partitionOffset: true, partitionLeader: true, partitionReplicas: true, partitionISRs: true, config: true} +var allFields = requestedTopicFields{ + partitionID: true, partitionOffset: true, partitionLeader: true, + partitionReplicas: true, partitionISRs: true, config: NonDefaultConfigs, +} type GetTopicsFlags struct { OutputFormat string @@ -60,8 +63,16 @@ type AlterTopicFlags struct { Configs []string } +type PrintConfigsParam string + +const ( + NoConfigs PrintConfigsParam = "none" + AllConfigs PrintConfigsParam = "all" + NonDefaultConfigs PrintConfigsParam = "no_defaults" +) + type DescribeTopicFlags struct { - PrintConfigs bool + PrintConfigs PrintConfigsParam SkipEmptyPartitions bool OutputFormat string } @@ -161,7 +172,10 @@ func (operation *Operation) DescribeTopic(topic string, flags DescribeTopicFlags return errors.Wrap(err, "failed to create cluster admin") } - if t, err = readTopic(&client, &admin, topic, allFields); err != nil { + fields := allFields + fields.config = flags.PrintConfigs + + if t, err = readTopic(&client, &admin, topic, fields); err != nil { return errors.Wrap(err, "failed to read topic") } @@ -170,7 +184,7 @@ func (operation *Operation) DescribeTopic(topic string, flags DescribeTopicFlags func (operation *Operation) printTopic(topic Topic, flags DescribeTopicFlags) error { - if !flags.PrintConfigs { + if flags.PrintConfigs == NoConfigs { topic.Configs = nil } @@ -401,7 +415,11 @@ func (operation *Operation) AlterTopic(topic string, flags AlterTopicFlags) erro } if flags.ValidateOnly { - describeFlags := DescribeTopicFlags{PrintConfigs: len(flags.Configs) > 0} + printConfigs := NoConfigs + if len(flags.Configs) > 0 { + printConfigs = NonDefaultConfigs + } + describeFlags := DescribeTopicFlags{PrintConfigs: printConfigs} return operation.printTopic(t, describeFlags) } return nil @@ -472,7 +490,7 @@ func (operation *Operation) CloneTopic(sourceTopic, targetTopic string) error { requestedFields := requestedTopicFields{ partitionID: true, partitionReplicas: true, - config: true, + config: NonDefaultConfigs, } if t, err = readTopic(&client, &admin, sourceTopic, requestedFields); err != nil { @@ -581,7 +599,7 @@ func (operation *Operation) GetTopics(flags GetTopicsFlags) error { } else if flags.OutputFormat == "compact" { tableWriter.Initialize() } else if flags.OutputFormat == "wide" { - requestedFields = requestedTopicFields{partitionID: true, partitionReplicas: true, config: true} + requestedFields = requestedTopicFields{partitionID: true, partitionReplicas: true, config: NonDefaultConfigs} if err := tableWriter.WriteHeader("TOPIC", "PARTITIONS", "REPLICATION FACTOR", "CONFIGS"); err != nil { return err } @@ -725,14 +743,14 @@ func readTopic(client *sarama.Client, admin *sarama.ClusterAdmin, name string, r return top.Partitions[i].ID < top.Partitions[j].ID }) - if requestedFields.config { + if requestedFields.config != NoConfigs { topicConfig := sarama.ConfigResource{ Type: sarama.TopicResource, Name: name, } - if top.Configs, err = internal.ListConfigs(admin, topicConfig); err != nil { + if top.Configs, err = internal.ListConfigs(admin, topicConfig, requestedFields.config == AllConfigs); err != nil { return top, err } }