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

Cleanup/imports parsing #509

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Conversation

smoser
Copy link
Contributor

@smoser smoser commented Sep 21, 2023

  • fix: Improvements to yaml.Unmarshal for types.Imports and add some go tests

    Improvements here:

    • Drop duplicate code blocks from getImportFromInterface
      There were 2 identical codeblocks, one with a type convert to
      map[string]interface{} and one with type convert to
      map[interface{}]interface{}. I cannot find any reason for the
      former
    • Add some golang tests to for unmarshalling.
    • Type check the values of all string Import fields, rather than just
      converting them to a string with 'fmt.Sprintf("%v")'
  • cleanup: Add contsants UidEmpty and GidEmpty and FileCopyNoPerms function.

    Instead of using '-1' to indicate "do not change userid" (or groupid)
    to FileCopy, provide constants UidEmpty and GidEmpty.

    Also add simplified FileCopyNoPerms to call FileCopy without mode
    or uid/gid.

@smoser smoser force-pushed the cleanup/imports-parsing branch 3 times, most recently from 6be6698 to fb633a3 Compare September 22, 2023 13:29
Instead of using '-1' to indicate "do not change userid" (or groupid)
to FileCopy, provide constants UidEmpty and GidEmpty.

Also add simplified FileCopyNoPerms to call FileCopy without mode
or uid/gid.

Signed-off-by: Scott Moser <smoser@brickies.net>
@smoser smoser force-pushed the cleanup/imports-parsing branch 5 times, most recently from bb66ba0 to 0aa3618 Compare September 23, 2023 00:03
… tests

There is one change here, and that is to break if a stacker yaml
has an import that is a dictionary type.  The content in an 'import'
is now only supported as a string or a list, not a dictionary.

For example, these are allowed:

 * import: /path/to/file

 * import:
    - /file
    - /file2
    - path: /file3

But this is not:

 * import:
     path: /file3

Improvements here:
 * Drop duplicate code blocks from getImportFromInterface
   There were 2 identical codeblocks, one with a type convert to
   map[string]interface{} and one with type convert to
   map[interface{}]interface{}.  That was present to handle the dict
   case.
 * Add some golang tests to for unmarshalling.
 * Type check the values of all string Import fields, rather than just
   converting them to a string with 'fmt.Sprintf("%v")'

Signed-off-by: Scott Moser <smoser@brickies.net>
Copy link
Contributor

@rchincha rchincha left a comment

Choose a reason for hiding this comment

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

lgtm

@rchincha
Copy link
Contributor

@mikemccracken @hallyn pls review.

@smoser smoser merged commit 96dbbe5 into project-stacker:main Oct 2, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants