Skip to content

Commit 5f4653c

Browse files
sanjimohsanjimoh
committed
address review comments
Signed-off-by: Sanjit Mohanty <[email protected]>
1 parent b91bc88 commit 5f4653c

File tree

3 files changed

+225
-59
lines changed

3 files changed

+225
-59
lines changed

‎acceptance/upload_product_test.go‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package acceptance
22

33
import (
44
"archive/zip"
5+
"fmt"
56
"io"
67
"net/http"
78
"os"
@@ -13,6 +14,8 @@ import (
1314
"github.com/onsi/gomega/gbytes"
1415
"github.com/onsi/gomega/gexec"
1516
"github.com/onsi/gomega/ghttp"
17+
"github.com/pivotal-cf/om/extractor"
18+
"github.com/pivotal-cf/om/validator"
1619
)
1720

1821
var _ = Describe("upload-product command", func() {
@@ -176,4 +179,212 @@ name: some-product`)
176179
})
177180
})
178181

182+
When("validating command flags", func() {
183+
var actualShasum string
184+
var actualVersion string
185+
var configFile *os.File
186+
var varsFile1 *os.File
187+
var varsFile2 *os.File
188+
189+
BeforeEach(func() {
190+
// Calculate the actual shasum of the test file
191+
shaValidator := validator.NewSHA256Calculator()
192+
var err error
193+
actualShasum, err = shaValidator.Checksum(productFile.Name())
194+
Expect(err).ToNot(HaveOccurred())
195+
196+
// Get the actual version from the test file
197+
metadataExtractor := extractor.NewMetadataExtractor()
198+
metadata, err := metadataExtractor.ExtractFromFile(productFile.Name())
199+
Expect(err).ToNot(HaveOccurred())
200+
actualVersion = metadata.Version
201+
202+
// Create temporary config file
203+
configFile, err = os.CreateTemp("", "config.yml")
204+
Expect(err).ToNot(HaveOccurred())
205+
_, err = configFile.WriteString("product: ((product_path))\npolling-interval: ((interval))")
206+
Expect(err).ToNot(HaveOccurred())
207+
Expect(configFile.Close()).To(Succeed())
208+
209+
// Create temporary vars files
210+
varsFile1, err = os.CreateTemp("", "vars1.yml")
211+
Expect(err).ToNot(HaveOccurred())
212+
_, err = varsFile1.WriteString(fmt.Sprintf("product_path: %s\ninterval: \"5\"", productFile.Name()))
213+
Expect(err).ToNot(HaveOccurred())
214+
Expect(varsFile1.Close()).To(Succeed())
215+
216+
varsFile2, err = os.CreateTemp("", "vars2.yml")
217+
Expect(err).ToNot(HaveOccurred())
218+
_, err = varsFile2.WriteString("additional_var: value")
219+
Expect(err).ToNot(HaveOccurred())
220+
Expect(varsFile2.Close()).To(Succeed())
221+
222+
// Add handler for product upload
223+
server.AppendHandlers(
224+
ghttp.CombineHandlers(
225+
ghttp.VerifyRequest("POST", "/api/v0/available_products"),
226+
http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
227+
err := req.ParseMultipartForm(100)
228+
Expect(err).ToNot(HaveOccurred())
229+
230+
requestFileName := req.MultipartForm.File["product[file]"][0].Filename
231+
Expect(requestFileName).To(Equal(filepath.Base(productFile.Name())))
232+
233+
w.WriteHeader(http.StatusOK)
234+
_, err = w.Write([]byte(`{}`))
235+
Expect(err).ToNot(HaveOccurred())
236+
}),
237+
),
238+
)
239+
})
240+
241+
AfterEach(func() {
242+
os.Remove(configFile.Name())
243+
os.Remove(varsFile1.Name())
244+
os.Remove(varsFile2.Name())
245+
})
246+
247+
It("accepts valid flags", func() {
248+
command := exec.Command(pathToMain,
249+
"--target", server.URL(),
250+
"--username", "some-username",
251+
"--password", "some-password",
252+
"--skip-ssl-validation",
253+
"upload-product",
254+
"--product", productFile.Name(),
255+
"--polling-interval", "5",
256+
"--shasum", actualShasum,
257+
"--product-version", actualVersion,
258+
)
259+
260+
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
261+
Expect(err).ToNot(HaveOccurred())
262+
Eventually(session, 5).Should(gexec.Exit(0))
263+
})
264+
265+
It("accepts valid short flags", func() {
266+
command := exec.Command(pathToMain,
267+
"--target", server.URL(),
268+
"--username", "some-username",
269+
"--password", "some-password",
270+
"--skip-ssl-validation",
271+
"upload-product",
272+
"-p", productFile.Name(),
273+
"-i", "5",
274+
)
275+
276+
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
277+
Expect(err).ToNot(HaveOccurred())
278+
Eventually(session, 5).Should(gexec.Exit(0))
279+
})
280+
281+
It("accepts all config interpolation flags", func() {
282+
command := exec.Command(pathToMain,
283+
"--target", server.URL(),
284+
"--username", "some-username",
285+
"--password", "some-password",
286+
"--skip-ssl-validation",
287+
"upload-product",
288+
"-c", configFile.Name(),
289+
"--vars-env", "MY",
290+
"-l", varsFile1.Name(),
291+
"-l", varsFile2.Name(),
292+
"-v", "FOO=bar",
293+
"-v", "BAZ=qux",
294+
"-p", productFile.Name(),
295+
)
296+
297+
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
298+
Expect(err).ToNot(HaveOccurred())
299+
Eventually(session, 5).Should(gexec.Exit(0))
300+
})
301+
302+
It("accepts mix of config and main flags", func() {
303+
command := exec.Command(pathToMain,
304+
"--target", server.URL(),
305+
"--username", "some-username",
306+
"--password", "some-password",
307+
"--skip-ssl-validation",
308+
"upload-product",
309+
"-p", productFile.Name(),
310+
"-c", configFile.Name(),
311+
"--vars-env", "MY",
312+
"--shasum", actualShasum,
313+
"-l", varsFile1.Name(),
314+
"-v", "FOO=bar",
315+
)
316+
317+
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
318+
Expect(err).ToNot(HaveOccurred())
319+
Eventually(session, 5).Should(gexec.Exit(0))
320+
})
321+
322+
It("rejects unknown flags", func() {
323+
command := exec.Command(pathToMain,
324+
"--target", server.URL(),
325+
"--username", "some-username",
326+
"--password", "some-password",
327+
"--skip-ssl-validation",
328+
"upload-product",
329+
"-p", productFile.Name(),
330+
"--notaflag",
331+
)
332+
333+
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
334+
Expect(err).ToNot(HaveOccurred())
335+
Eventually(session, 5).Should(gexec.Exit(1))
336+
Eventually(session.Err).Should(gbytes.Say("Error: unknown flag\\(s\\) \\[\"--notaflag\"\\] for command 'upload-product'"))
337+
})
338+
339+
It("rejects multiple unknown flags", func() {
340+
command := exec.Command(pathToMain,
341+
"--target", server.URL(),
342+
"--username", "some-username",
343+
"--password", "some-password",
344+
"--skip-ssl-validation",
345+
"upload-product",
346+
"--foo",
347+
"--bar",
348+
)
349+
350+
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
351+
Expect(err).ToNot(HaveOccurred())
352+
Eventually(session, 5).Should(gexec.Exit(1))
353+
Eventually(session.Err).Should(gbytes.Say("Error: unknown flag\\(s\\) \\[\"--foo\" \"--bar\"\\] for command 'upload-product'"))
354+
})
355+
356+
It("rejects unknown short flags", func() {
357+
command := exec.Command(pathToMain,
358+
"--target", server.URL(),
359+
"--username", "some-username",
360+
"--password", "some-password",
361+
"--skip-ssl-validation",
362+
"upload-product",
363+
"-p", productFile.Name(),
364+
"-x", "18000",
365+
)
366+
367+
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
368+
Expect(err).ToNot(HaveOccurred())
369+
Eventually(session, 5).Should(gexec.Exit(1))
370+
Eventually(session.Err).Should(gbytes.Say("Error: unknown flag\\(s\\) \\[\"-x\"\\] for command 'upload-product'"))
371+
})
372+
373+
It("rejects another unknown short flag", func() {
374+
command := exec.Command(pathToMain,
375+
"--target", server.URL(),
376+
"--username", "some-username",
377+
"--password", "some-password",
378+
"--skip-ssl-validation",
379+
"upload-product",
380+
"-p", productFile.Name(),
381+
"-z", "18000",
382+
)
383+
384+
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)
385+
Expect(err).ToNot(HaveOccurred())
386+
Eventually(session, 5).Should(gexec.Exit(1))
387+
Eventually(session.Err).Should(gbytes.Say("Error: unknown flag\\(s\\) \\[\"-z\"\\] for command 'upload-product'"))
388+
})
389+
})
179390
})

‎cmd/main.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ func Main(sout io.Writer, serr io.Writer, version string, applySleepDurationStri
458458
_, err = parser.AddCommand(
459459
"generate-certificate",
460460
"generates a new certificate signed by Ops Manager's root CA",
461-
"This authenticated command generates a new RSA public/private certificate signed by Ops Manager’s root CA certificate",
461+
"This authenticated command generates a new RSA public/private certificate signed by Ops Manager's root CA certificate",
462462
commands.NewGenerateCertificate(api, stdout),
463463
)
464464
if err != nil {
@@ -809,27 +809,31 @@ func validateCommandFlags(parser *flags.Parser, args []string) error {
809809

810810
var selectedCmd *flags.Command
811811
cmds := parser.Commands()
812-
argIdx := 0
812+
// currentArgPosition tracks our position in the args array as we traverse the command hierarchy
813+
currentArgPosition := 0
813814

814815
// Find the top-level command
815816
for _, cmd := range cmds {
816-
if cmd.Name == args[argIdx] || contains(cmd.Aliases, args[argIdx]) {
817+
if cmd.Name == args[currentArgPosition] || contains(cmd.Aliases, args[currentArgPosition]) {
817818
selectedCmd = cmd
818819
break
819820
}
820821
}
821822
if selectedCmd == nil {
822823
return nil // unknown command, let parser handle it
823824
}
824-
argIdx++
825+
currentArgPosition++
825826

826827
// Walk down subcommands as long as the next arg matches a subcommand
827-
for argIdx < len(args) {
828+
// For example: "om vm-lifecycle export-opsman-config" would traverse:
829+
// 1. "vm-lifecycle" (top-level command)
830+
// 2. "export-opsman-config" (subcommand)
831+
for currentArgPosition < len(args) {
828832
found := false
829833
for _, sub := range selectedCmd.Commands() {
830-
if sub.Name == args[argIdx] || contains(sub.Aliases, args[argIdx]) {
834+
if sub.Name == args[currentArgPosition] || contains(sub.Aliases, args[currentArgPosition]) {
831835
selectedCmd = sub
832-
argIdx++
836+
currentArgPosition++
833837
found = true
834838
break
835839
}
@@ -840,12 +844,13 @@ func validateCommandFlags(parser *flags.Parser, args []string) error {
840844
}
841845

842846
// Now selectedCmd is the deepest subcommand
843-
invalidFlags := findUnknownFlags(selectedCmd, args[argIdx:])
847+
// Check remaining args for unknown flags
848+
invalidFlags := findUnknownFlags(selectedCmd, args[currentArgPosition:])
844849

845850
if len(invalidFlags) > 0 {
846851
fmt.Fprintf(os.Stderr, "Error: unknown flag(s) %q for command '%s'\n", invalidFlags, selectedCmd.Name)
847852
fmt.Fprintf(os.Stderr, "See 'om %s --help' for available options.\n", selectedCmd.Name)
848-
return fmt.Errorf("unknown flag(s) %q for command '%s'", invalidFlags, selectedCmd.Name)
853+
os.Exit(1)
849854
}
850855
return nil
851856
}

‎cmd/validate_command_flags_test.go‎

Lines changed: 0 additions & 50 deletions
This file was deleted.

0 commit comments

Comments
 (0)