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

Add validation for output file path / name #386

Merged
merged 1 commit into from
Nov 4, 2021

Conversation

szh
Copy link
Contributor

@szh szh commented Nov 3, 2021

Note: This depends on #381

Implemented Changes

Implement validation on the annotation specifying output file path / name and make sure there are tests for the following cases:

  • File name must be unique across secret groups (including secret groups that are using default file paths)
  • The path is a valid Linux filename (check length)

Also ensure that if you are using a non-custom (i.e. non-templatized) file format (or using the default yaml template), and:

  • If you don't specify the output file path annotation, then you get a default filepath of the form <secret-group-name>.<file-type>.
  • If you do specify the output file path annotation, this (explicit) output file path annotation takes precedence over the default output file path for that file format (i.e. BOTH the file directory and file name are overridden).

Connected Issue/Story

CyberArk internal issue link: ONYX-13143

Changelog

  • The CHANGELOG has been updated, or
  • This PR does not include user-facing changes and doesn't require a
    CHANGELOG update

Test coverage

  • This PR includes new unit and integration tests to go with the code
    changes, or
  • The changes in this PR do not require tests

Documentation

  • Docs (e.g. READMEs) were updated in this PR
  • A follow-up issue to update official docs has been filed here: insert issue ID
  • This PR does not require updating any documentation

Behavior

  • This PR changes product behavior and has been reviewed by a PO, or
  • These changes are part of a larger initiative that will be reviewed later, or
  • No behavior was changed with this PR

Security

  • Security architect has reviewed the changes in this PR,
  • These changes are part of a larger initiative with a separate security review, or
  • There are no security aspects to these changes

@szh szh self-assigned this Nov 3, 2021
@szh szh requested a review from john-odonnell November 3, 2021 19:47
@szh szh changed the title Validate output path Add validation for output file path / name Nov 3, 2021
pkg/secrets/pushtofile/secret_group.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/secret_group.go Outdated Show resolved Hide resolved
@szh szh force-pushed the ONYX-13143-validate-output-path branch from 0c77622 to 3c58283 Compare November 4, 2021 14:32
fileTemplate,
secrets,
)
}

func (sg *SecretGroup) absoluteFilePath(secretsBasePath string) (string, error) {
Copy link

Choose a reason for hiding this comment

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

Method SecretGroup.absoluteFilePath has 5 return statements (exceeds 4 allowed).

@szh szh force-pushed the ONYX-13143-validate-output-path branch 2 times, most recently from d6acb31 to 0a399e7 Compare November 4, 2021 15:38
@szh szh marked this pull request as ready for review November 4, 2021 15:42
@szh szh requested review from a team as code owners November 4, 2021 15:42
Copy link
Contributor

@doodlesbykumbi doodlesbykumbi left a comment

Choose a reason for hiding this comment

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

PR looking good. Left some comments

pkg/secrets/pushtofile/secret_group.go Show resolved Hide resolved
pkg/secrets/pushtofile/secret_group.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/secret_group.go Outdated Show resolved Hide resolved
@szh szh force-pushed the ONYX-13143-validate-output-path branch 2 times, most recently from 6961df4 to 5e21452 Compare November 4, 2021 17:56
@szh szh force-pushed the ONYX-13143-validate-output-path branch from 5e21452 to a9a47ee Compare November 4, 2021 18:52
@szh szh force-pushed the ONYX-13143-validate-output-path branch from a9a47ee to 02e7fcb Compare November 4, 2021 19:13
@codeclimate
Copy link

codeclimate bot commented Nov 4, 2021

Code Climate has analyzed commit 02e7fcb and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 90.4% (0.3% change).

View more on Code Climate.

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.

3 participants