Skip to content

Commit 9849ced

Browse files
a-spikerdwelch-spike
a-spiker
and
dwelch-spike
authored
feat: TOOLS-3077 - Add Server Diff Functionality to asconfig diff Command (#70)
* feat: TOOLS-3077 add server diff functionality to compare local config with running Aerospike server - Introduced a new `--server` flag for the `diff` command to allow comparison of a local configuration file against the live configuration from a running Aerospike server. - Updated error handling for argument counts specific to server diff. - Refactored the diff command to separate file-to-file and server-to-file comparison logic. * test: add comprehensive tests for server diff functionality and argument validation - Introduced new test cases for the `--server` flag in the diff command, covering various scenarios including missing arguments, invalid formats, and non-existent files. - Added validation tests for server diff arguments to ensure proper error handling. - Enhanced existing tests for file diff functionality to include edge cases and argument validation. * chore: update go.sum to remove obsolete aerospike-management-lib entries - Removed outdated versions of aerospike-management-lib from go.sum to streamline dependencies and ensure compatibility with the latest library versions. * feat: Improve type-aware comparison in diff logic Replaces reflect.DeepEqual with a custom valuesEqual function for more robust type-aware comparison, handling cases like int vs string, slices, and booleans. Ensures server and local configs are parsed through the same path to normalize data types before diffing, improving accuracy of configuration diffs. * fix: Improve valuesEqual logic and add comprehensive tests Refactored the valuesEqual function to compare values by string conversion first, improving type flexibility and preventing panics on slice comparison. Added extensive unit tests for valuesEqual, isSlice, slicesEqual, diffFlatMaps, and argument validation in diff_test.go to ensure correctness and robustness of diff logic. * feat: make slice comparison order-agnostic and efficient Refactored slicesEqual to compare slices in an order-agnostic way using frequency counting for O(n) complexity. Added convertToStringSlice to handle different slice types and ensure consistent comparison. Updated tests to validate new behavior, including mixed types and order variations. * feat: Add beta warning and adjust log levels in diff command Introduces a warning message indicating the server diff feature is in beta Since the generate command is in beta and diff with server uses generate functionality * refactor: refactor diff command and add subcommand for server diff Refactors the diff command to use a subcommand structure, moving server diff functionality to a new 'diff server' subcommand. Simplifies flag handling, updates help text and examples, and streamlines the diff logic by removing custom value comparison in favor of reflect.DeepEqual. * refactor: Remove redundant and low-level unit tests from diff_test.go This commit removes extensive low-level unit tests for helper functions such as valuesEqual, isSlice, and slicesEqual, as well as redundant server diff argument validation and flag validation tests. The remaining tests focus on higher-level command behavior and integration, streamlining the test suite and reducing maintenance overhead. * docs: Update diff command descriptions for server comparison Expanded the short and long descriptions of the diff command to clarify support for comparing a config file against a running server. Marked the server comparison feature as BETA in the server subcommand descriptions. * docs: TOOLS-3077 Update diff command descriptions for clarity Improved the short and long descriptions for the diff and server subcommands to clarify that comparisons are made against a running server's configuration. Updated examples and added notes about supported file formats for better user guidance. * fix: Refactor format flag handling in diff commands Moved the --format flag from persistent to local flags in the diff command and hid it in the server diff subcommand, as the format will be auto-detected. Added a debug log to show the detected local file format. * feat: TOOLS-3077 Refactor diff command to use subcommands Introduces 'files' and 'server' subcommands for the diff command, improving clarity and extensibility. The legacy behavior is preserved for backward compatibility, with a warning message. Adds tests for both legacy and new subcommand usage, including argument validation. * fix: Update diff tests to use substring match Changed the diff test assertion to check if the output contains the expected result instead of requiring an exact match. This allows for additional warnings or messages in the output without failing the test. * docs: Update diff command help text and examples Improved the help text and examples for the diff, diff files, and diff server commands to clarify usage, supported formats, and default behaviors. Added more detailed flag descriptions and updated example host/port formats for consistency. * Update cmd/diff.go update server diff usage example Co-authored-by: dwelch-spike <[email protected]> --------- Co-authored-by: dwelch-spike <[email protected]>
1 parent 61fa9aa commit 9849ced

File tree

3 files changed

+599
-86
lines changed

3 files changed

+599
-86
lines changed

‎cmd/diff.go‎

Lines changed: 254 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,27 @@ import (
99
"strings"
1010

1111
asConf "github.com/aerospike/aerospike-management-lib/asconfig"
12+
"github.com/aerospike/aerospike-management-lib/info"
13+
"github.com/aerospike/asconfig/conf"
14+
"github.com/aerospike/tools-common-go/config"
15+
"github.com/aerospike/tools-common-go/flags"
1216

1317
"github.com/spf13/cobra"
1418
)
1519

1620
const (
17-
diffArgMin = 2
18-
diffArgMax = 2
21+
diffArgMin = 2
22+
diffArgMax = 2
23+
diffServerArgMin = 1 // For server diff, we need only one local file
24+
diffServerArgMax = 1
1925
)
2026

2127
var (
22-
errDiffConfigsDiffer = errors.Join(fmt.Errorf("configuration files are not equal"), ErrSilent)
23-
errDiffTooFewArgs = fmt.Errorf("diff requires atleast %d file paths as arguments", diffArgMin)
24-
errDiffTooManyArgs = fmt.Errorf("diff requires no more than %d file paths as arguments", diffArgMax)
28+
errDiffConfigsDiffer = errors.Join(fmt.Errorf("configuration files are not equal"), ErrSilent)
29+
errDiffTooFewArgs = fmt.Errorf("diff requires atleast %d file paths as arguments", diffArgMin)
30+
errDiffTooManyArgs = fmt.Errorf("diff requires no more than %d file paths as arguments", diffArgMax)
31+
errDiffServerTooFewArgs = fmt.Errorf("diff with --server requires exactly %d file path as argument", diffServerArgMin)
32+
errDiffServerTooManyArgs = fmt.Errorf("diff with --server requires no more than %d file path as argument", diffServerArgMax)
2533
)
2634

2735
func init() {
@@ -32,95 +40,254 @@ var diffCmd = newDiffCmd()
3240

3341
func newDiffCmd() *cobra.Command {
3442
res := &cobra.Command{
35-
Use: "diff [flags] <path/to/config1> <path/to/config2>",
36-
Short: "Diff yaml or conf Aerospike configuration files.",
37-
Long: `Diff is used to compare differences between Aerospike configuration files.
38-
It is used on two files of the same format from any format
39-
supported by the asconfig tool, e.g. yaml or Aerospike config.
40-
Schema validation is not performed on either file. The file names must end with
41-
extensions signifying their formats, e.g. .conf or .yaml, or --format must be used.`,
43+
Use: "diff",
44+
Short: "Diff Aerospike configuration files or a file against a running server's configuration.",
45+
Long: `Diff is used to compare Aerospike configuration files, or a file against a running server's configuration.
46+
47+
If no subcommand is provided, 'files' is used by default for backward compatibility.
48+
49+
See subcommands for available diff modes.`,
50+
Example: `
51+
# Diff two local yaml configuration files
52+
asconfig diff files aerospike1.yaml aerospike2.yaml
53+
# Diff a local .conf file against a running server
54+
asconfig diff server -h 127.0.0.1:3000 aerospike.conf`,
4255
RunE: func(cmd *cobra.Command, args []string) error {
43-
logger.Debug("Running diff command")
44-
45-
if len(args) < diffArgMin {
46-
return errDiffTooFewArgs
47-
}
48-
49-
if len(args) > diffArgMax {
50-
return errDiffTooManyArgs
51-
}
52-
53-
path1 := args[0]
54-
path2 := args[1]
55-
56-
logger.Debugf("Diff file 1 is %s", path1)
57-
logger.Debugf("Diff file 2 is %s", path2)
58-
59-
fmt1, err := getConfFileFormat(path1, cmd)
60-
if err != nil {
61-
return err
62-
}
63-
64-
fmt2, err := getConfFileFormat(path2, cmd)
65-
if err != nil {
66-
return err
67-
}
68-
69-
logger.Debugf("Diff file 1 format is %v", fmt1)
70-
logger.Debugf("Diff file 2 format is %v", fmt2)
71-
72-
if fmt1 != fmt2 {
73-
return fmt.Errorf("mismatched file formats, detected %s and %s", fmt1, fmt2)
74-
}
75-
76-
f1, err := os.ReadFile(path1)
77-
if err != nil {
78-
return err
79-
}
80-
81-
f2, err := os.ReadFile(path2)
82-
if err != nil {
83-
return err
84-
}
85-
86-
// not performing any validation so server version is "" (not needed)
87-
// won't be marshaling these configs to text so use Invalid output format
88-
// TODO decouple output format from asconf, probably pass it as an
89-
// arg to marshal text
90-
conf1, err := asConf.NewASConfigFromBytes(mgmtLibLogger, f1, fmt1)
91-
if err != nil {
92-
return err
93-
}
94-
95-
conf2, err := asConf.NewASConfigFromBytes(mgmtLibLogger, f2, fmt2)
96-
if err != nil {
97-
return err
98-
}
99-
100-
// get flattened config maps
101-
map1 := conf1.GetFlatMap()
102-
map2 := conf2.GetFlatMap()
103-
104-
diffs := diffFlatMaps(
105-
*map1,
106-
*map2,
107-
)
108-
109-
if len(diffs) > 0 {
110-
fmt.Printf("Differences shown from %s to %s, '<' are from file1, '>' are from file2.\n", path1, path2)
111-
fmt.Println(strings.Join(diffs, ""))
112-
return errDiffConfigsDiffer
113-
}
114-
115-
return nil
56+
logger.Warn("Using legacy 'diff' subcommand. Use 'diff files' instead.")
57+
return runFileDiff(cmd, args)
11658
},
11759
}
11860

61+
res.Version = VERSION
11962
res.Flags().StringP("format", "F", "conf", "The format of the source file(s). Valid options are: yaml, yml, and conf.")
12063

64+
// Add subcommands
65+
res.AddCommand(newDiffFilesCmd())
66+
res.AddCommand(newDiffServerCmd())
67+
12168
return res
12269
}
12370

71+
// newDiffFilesCmd creates the 'diff files' subcommand (the legacy default)
72+
func newDiffFilesCmd() *cobra.Command {
73+
cmd := &cobra.Command{
74+
Use: "files [flags] <path/to/config1> <path/to/config2>",
75+
Short: "Diff yaml or conf Aerospike configuration files.",
76+
Long: `Diff is used to compare differences between two Aerospike configuration files.
77+
It is used on two files of the same format from any format
78+
supported by the asconfig tool, e.g. yaml or Aerospike config.
79+
Schema validation is not performed on either file. The file names must end with
80+
extensions signifying their formats, e.g. .conf or .yaml, or --format must be used.`,
81+
Example: `
82+
# Compare two local configuration files
83+
asconfig diff files aerospike1.conf aerospike2.conf
84+
# Compare two local yaml configuration files
85+
asconfig diff files --format yaml aerospike1.yaml aerospike2.yaml`,
86+
RunE: func(cmd *cobra.Command, args []string) error {
87+
logger.Debug("Running diff files command")
88+
return runFileDiff(cmd, args)
89+
},
90+
}
91+
cmd.Version = VERSION
92+
cmd.Flags().StringP("format", "F", "conf", "The format of the source file(s). Valid options are: yaml, yml, and conf.")
93+
return cmd
94+
}
95+
96+
func newDiffServerCmd() *cobra.Command {
97+
cmd := &cobra.Command{
98+
Use: "server [flags] <path/to/config>",
99+
Short: "BETA: Diff a local config file against a running Aerospike server's configuration.",
100+
Long: `BETA: Diff is used to compare a local configuration file against the configuration of a running Aerospike server.
101+
This is useful for spotting drift between expected and actual Aerospike server configurations.
102+
In this mode, only one config file path is required as an argument.
103+
Note: The configuration file can be in yaml or conf format.`,
104+
Example: `Diff a local .conf file against a running server
105+
asconfig diff server -h 127.0.0.1:3000 aerospike.conf`,
106+
RunE: func(cmd *cobra.Command, args []string) error {
107+
logger.Debug("Running server diff command")
108+
return runServerDiff(cmd, args)
109+
},
110+
}
111+
112+
// Add format flag but hide it from help output as it will be automatically detected
113+
cmd.Flags().StringP("format", "F", "conf", "The format of the source file(s). Valid options are: yaml, yml, and conf.")
114+
cmd.Flags().MarkHidden("format")
115+
116+
// Add Aerospike connection flags when server mode is enabled
117+
asFlagSet := aerospikeFlags.NewFlagSet(flags.DefaultWrapHelpString)
118+
cmd.Flags().AddFlagSet(asFlagSet)
119+
config.BindPFlags(asFlagSet, "cluster")
120+
121+
return cmd
122+
}
123+
124+
// runFileDiff handles the original file-to-file diff functionality
125+
func runFileDiff(cmd *cobra.Command, args []string) error {
126+
if len(args) < diffArgMin {
127+
return errDiffTooFewArgs
128+
}
129+
130+
if len(args) > diffArgMax {
131+
return errDiffTooManyArgs
132+
}
133+
134+
path1 := args[0]
135+
path2 := args[1]
136+
137+
logger.Debugf("Diff file 1 is %s", path1)
138+
logger.Debugf("Diff file 2 is %s", path2)
139+
140+
fmt1, err := getConfFileFormat(path1, cmd)
141+
if err != nil {
142+
return err
143+
}
144+
145+
fmt2, err := getConfFileFormat(path2, cmd)
146+
if err != nil {
147+
return err
148+
}
149+
150+
logger.Debugf("Diff file 1 format is %v", fmt1)
151+
logger.Debugf("Diff file 2 format is %v", fmt2)
152+
153+
if fmt1 != fmt2 {
154+
return fmt.Errorf("mismatched file formats, detected %s and %s", fmt1, fmt2)
155+
}
156+
157+
f1, err := os.ReadFile(path1)
158+
if err != nil {
159+
return err
160+
}
161+
162+
f2, err := os.ReadFile(path2)
163+
if err != nil {
164+
return err
165+
}
166+
167+
// not performing any validation so server version is "" (not needed)
168+
// won't be marshaling these configs to text so use Invalid output format
169+
// TODO decouple output format from asconf, probably pass it as an
170+
// arg to marshal text
171+
conf1, err := asConf.NewASConfigFromBytes(mgmtLibLogger, f1, fmt1)
172+
if err != nil {
173+
return err
174+
}
175+
176+
conf2, err := asConf.NewASConfigFromBytes(mgmtLibLogger, f2, fmt2)
177+
if err != nil {
178+
return err
179+
}
180+
181+
// get flattened config maps
182+
map1 := conf1.GetFlatMap()
183+
map2 := conf2.GetFlatMap()
184+
185+
diffs := diffFlatMaps(
186+
*map1,
187+
*map2,
188+
)
189+
190+
if len(diffs) > 0 {
191+
fmt.Printf("Differences shown from %s to %s, '<' are from file1, '>' are from file2.\n", path1, path2)
192+
fmt.Println(strings.Join(diffs, ""))
193+
return errDiffConfigsDiffer
194+
}
195+
196+
return nil
197+
}
198+
199+
// runServerDiff handles comparing a local file against a running server
200+
func runServerDiff(cmd *cobra.Command, args []string) error {
201+
if len(args) < diffServerArgMin {
202+
return errDiffServerTooFewArgs
203+
}
204+
205+
if len(args) > diffServerArgMax {
206+
return errDiffServerTooManyArgs
207+
}
208+
209+
logger.Warning(
210+
"This feature is currently in beta. Use at your own risk and please report any issue to support.",
211+
)
212+
213+
localPath := args[0]
214+
logger.Debugf("Comparing local file %s against server", localPath)
215+
216+
// Get local file format and content
217+
localFormat, err := getConfFileFormat(localPath, cmd)
218+
if err != nil {
219+
return err
220+
}
221+
222+
logger.Debugf("Local file format is %v", localFormat)
223+
224+
localFile, err := os.ReadFile(localPath)
225+
if err != nil {
226+
return err
227+
}
228+
229+
// Create local config
230+
localConf, err := asConf.NewASConfigFromBytes(mgmtLibLogger, localFile, localFormat)
231+
if err != nil {
232+
return err
233+
}
234+
235+
logger.Debugf("Generating config from Aerospike node")
236+
// Generate server config using existing generate functionality
237+
asCommonConfig := aerospikeFlags.NewAerospikeConfig()
238+
239+
asPolicy, err := asCommonConfig.NewClientPolicy()
240+
if err != nil {
241+
return errors.Join(fmt.Errorf("unable to create client policy"), err)
242+
}
243+
244+
logger.Debugf("Retrieving Aerospike configuration from server")
245+
246+
asHosts := asCommonConfig.NewHosts()
247+
asinfo := info.NewAsInfo(mgmtLibLogger, asHosts[0], asPolicy)
248+
249+
generatedConf, err := asConf.GenerateConf(mgmtLibLogger, asinfo, true)
250+
if err != nil {
251+
return errors.Join(fmt.Errorf("unable to generate config from server"), err)
252+
}
253+
254+
// Convert server config to the same format as local file to ensure same parsing path
255+
serverConfHandler, err := asConf.NewMapAsConfig(mgmtLibLogger, generatedConf.Conf)
256+
if err != nil {
257+
return errors.Join(fmt.Errorf("unable to parse the generated server conf"), err)
258+
}
259+
260+
// Marshal server config to bytes in the same format as local file
261+
serverConfigMarshaller := conf.NewConfigMarshaller(serverConfHandler, localFormat)
262+
serverConfigBytes, err := serverConfigMarshaller.MarshalText()
263+
if err != nil {
264+
return errors.Join(fmt.Errorf("unable to marshal server config"), err)
265+
}
266+
267+
// Parse server config bytes using the same path as local file
268+
serverConf, err := asConf.NewASConfigFromBytes(mgmtLibLogger, serverConfigBytes, localFormat)
269+
if err != nil {
270+
return errors.Join(fmt.Errorf("unable to parse server config bytes"), err)
271+
}
272+
273+
// Get flattened config maps - now both should have the same data types
274+
localMap := localConf.GetFlatMap()
275+
serverMap := serverConf.GetFlatMap()
276+
277+
diffs := diffFlatMaps(
278+
*localMap,
279+
*serverMap,
280+
)
281+
282+
if len(diffs) > 0 {
283+
fmt.Printf("Differences shown from %s to server, '<' are from local file, '>' are from server.\n", localPath)
284+
fmt.Println(strings.Join(diffs, ""))
285+
return errDiffConfigsDiffer
286+
}
287+
288+
return nil
289+
}
290+
124291
// diffFlatMaps reports differences between flattened config maps
125292
// this only works for maps 1 layer deep as produced by the management
126293
// lib's flattenConf function
@@ -168,6 +335,8 @@ func diffFlatMaps(m1 map[string]any, m2 map[string]any) []string {
168335
}
169336

170337
if !reflect.DeepEqual(v1, v2) {
338+
// Debug: print types and values for investigation
339+
logger.Debugf("Diff found for key '%s': local=%v (type=%T), server=%v (type=%T)", k, v1, v1, v2, v2)
171340
res = append(res, fmt.Sprintf("%s:\n\t<: %v\n\t>: %v\n", k, v1, v2))
172341
}
173342
}

0 commit comments

Comments
 (0)