Skip to content

Mchinta/refactor - #336

Merged
Tommy Barnes (tbarnes94) merged 44 commits into
staging-kustomizehelmfrom
mchinta/refactor
Jul 24, 2024
Merged

Mchinta/refactor#336
Tommy Barnes (tbarnes94) merged 44 commits into
staging-kustomizehelmfrom
mchinta/refactor

Conversation

@manasachi

@manasachi Manasa Chinta (manasachi) commented Jul 18, 2024

Copy link
Copy Markdown
Contributor

Description

  • Added GetManifestFiles function which gets a list of ManifestFiles based on whether given path is a Helm or Kustomize Directory/file, a dir of manifest files, or path to a single manifest file. Note: The existing GetManifestFiles function is renamed to GetManifestFilesFromDir(), so no breaking changes can be introduced in validate.go
  • Removes those existing checks from validate.go run().
  • Tests will be improved once ManifestFile struct Path property is updated to hold file content as []byte

Fixes # (issue)
Feature # (details)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Refactoring

How Has This Been Tested?

Tested that validate.go does not break with removing code from run(). Added separate test cases for GetManifestFiles()

  • Test A
  • Test B

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Vidya Reddy (Vidya2606) and others added 8 commits June 20, 2024 15:32
Co-authored-by: Vidya Reddy <59590642+Vidya2606@users.noreply.github.com>
Signed-off-by: manasachi <manasachinta7@gmail.com>
Co-authored-by: Vidya Reddy <59590642+Vidya2606@users.noreply.github.com>
Co-authored-by: Vidya Reddy <59590642+Vidya2606@users.noreply.github.com>
Co-authored-by: David Gamero <david340804@gmail.com>
Co-authored-by: David Gamero <david340804@gmail.com>

@tbarnes94 Tommy Barnes (tbarnes94) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great changes!!

Comment thread cmd/validate.go Outdated
Comment thread cmd/validate_test.go Outdated
)

// Given a path, will determine if it's Kustomize, Helm, a directory of manifests, or a single manifest
func GetManifestFiles(manifestsPath string) ([]safeguards.ManifestFile, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where is this function called? You have some repeated logic here and in GetManifestFilesFromDir

@manasachi Manasa Chinta (manasachi) Jul 23, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

getManifestFilesFromDir is used to recursively process a directory of .yaml files. And GetManfiestFiles() calls this func, in addition to handling helm, kustomize, single .yaml cases. I left it like this since RenderHelm and RenderKustomize functions handle processing directories differently. If this is too much overlap, we could look into refactoring all functions in anther PR. However, at the moment, I don't see how we could reduce code between these two functions.

Comment thread pkg/safeguards/preprocessing/preprocessing_helpers.go Outdated
Comment thread pkg/safeguards/preprocessing/preprocessing_test.go Outdated
Suneha Bose (bosesuneha) and others added 7 commits July 23, 2024 15:01
Signed-off-by: manasachi <manasachinta7@gmail.com>
Signed-off-by: manasachi <manasachinta7@gmail.com>
Signed-off-by: manasachi <manasachinta7@gmail.com>
Signed-off-by: manasachi <manasachinta7@gmail.com>
Signed-off-by: manasachi <manasachinta7@gmail.com>
@manasachi
Manasa Chinta (manasachi) changed the base branch from main to staging-kustomizehelm July 24, 2024 18:32
@tbarnes94
Tommy Barnes (tbarnes94) merged commit 02c0f12 into staging-kustomizehelm Jul 24, 2024
@tbarnes94
Tommy Barnes (tbarnes94) deleted the mchinta/refactor branch July 24, 2024 18:32
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.

6 participants