feat(file): implement `spec.files` for `kind: file` by lemeurherve · Pull Request #490 · updatecli/updatecli

lemeurherve · 2022-01-26T17:50:21Z

Ability to specify more than one file for `kind: file`

Fixes #435

Additional validations

General:

allow only one file if spec.Line is specified

For sources & conditions

allow only one file

For targets:

allow multiple files if no spec.Line is specified

Test

To test this pull request, you can run the following commands:

cp <to_package_directory>
go test

Additional Information

spec.file should be deprecated in the following version.

Potential improvements

do the same for kind: dockerfile and kind: yaml
Add tests with "file://..."
Rename the package file to files

lemeurherve · 2022-01-30T15:44:50Z

file.yaml: success :)

lemeurherve · 2022-01-31T00:45:38Z

I didn't found the report of CodeQL:

… it's already checked at the spec validation

olblak · 2022-03-30T15:21:53Z

I think I spot an error with the following manifest when using the line from a source

Well it can't reproduce it :/

olblak · 2022-03-30T15:39:22Z

I think I spot an error with the following manifest when using the line from a source

Well it can't reproduce it :/

Well I can randomly reproduce it

olblak · 2022-03-30T15:43:39Z

The result of my experimentation
https://gist.github.com/olblak/88351cf2047520abe44eefb65380abdf

lemeurherve · 2022-03-31T08:20:29Z

I've got this error with your manifest: https://gist.github.com/lemeurherve/4d00d36115c170a40899f8d699abbb91

lemeurherve · 2022-04-02T12:52:01Z

@olblak reproduced and fixed :)

Please also review lemeurherve#18 cf this comment ;)

olblak · 2022-04-04T07:08:37Z

Thanks @lemeurherve it works great.
I am waiting for @dduportal to provide a final feedback before merging this pullrequest.

Also so, would you have some time to add a manifest to test the different "file" usecases in the e2e directory?

Configuration cannot have WARNING or Error as defined in the venom test here

olblak · 2022-04-05T09:21:24Z

@lemeurherve linting test are failing :)

lemeurherve · 2022-04-05T18:55:05Z

Looking into e2e errors

Co-authored-by: Olivier Vernin <olivier@vernin.me>

lemeurherve · 2022-04-06T13:18:36Z

The e2e error comes from udpatecli version:

$ dist/updatecli_darwin_amd64/updatecli version

VERSION

Application:

Build Time :

$ updatecli version

VERSION

Application:    0.22.3
Golang     :    1.17.8 linux/amd64
Build Time :    2022-03-22T09:52:05Z

Not sure why though

olblak · 2022-04-06T13:32:21Z

The e2e error comes from udpatecli version:

$ dist/updatecli_darwin_amd64/updatecli version

VERSION

Application:

Build Time :

$ updatecli version

VERSION

Application:    0.22.3
Golang     :    1.17.8 linux/amd64
Build Time :    2022-03-22T09:52:05Z

Not sure why though

It depends who you build updatecli locally, because you may not have set the flag that define the different version information

lemeurherve · 2022-04-06T13:33:03Z

OK. Do the e2e tests pass locally for you?

olblak · 2022-04-06T14:08:20Z

OK. Do the e2e tests pass locally for you?

Yes it does

 • Updatecli Version TestSuite (e2e/venom.d/test_version.yaml)
 	• Test-updatecli-version-return-code-is-0 SUCCESS

olblak · 2022-04-06T14:08:54Z

Maybe @dduportal can help you in case of something specific on mac which I am missing

olblak · 2022-04-06T14:22:13Z

Anyway e2e test are executed for each pullrequest in the build workflow so you'll know very quickly if something goes wrong. :D

dduportal

LGTM 🚀

Follow-up of updatecli#490 Ref updatecli#435

* feat(yaml): implement spec.files for kind: yaml Follow-up of #490 Ref #435 * Add e2e test for yaml (#43) * Apply suggestions from code review Co-authored-by: Olivier Vernin <olivier@vernin.me> * chore: move test to e2e success folder * wip: first stab at files for condition too * Revert "wip: first stab at files for condition too" This reverts commit 957f21e. * restore comment, will be done in #729 * chore: replace deprecated sourceID field by sourceid * fix: e2e tests * breaking?: no error if the target content hasn't changed * Remove broken test file Signed-off-by: Olblak <me@olblak.com> * patch from main (with #846 included and adapted) * fix tests * Add missing jsonschema annotations Co-authored-by: Olivier Vernin <olivier@vernin.me> * fix: remove files field from expected autodiscovery tests results Omited if empty since previous commit * cleanup: remove non mandatory empty files field from chart examples * chore: more complete debug log with initial filePath * chore: add debug logs * Update pkg/plugins/resources/yaml/main.go * chore: add missing Test_TargetFromSCM test * Update pkg/plugins/resources/yaml/source.go Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com> * Update pkg/plugins/resources/yaml/main.go Co-authored-by: Olivier Vernin <olivier@vernin.me> * allow multiples files for 'condition' * update example with a condition checking multiple files * remove TODOs * remove unrelated changes * replace example by e2e test * correct check for s.file * Update pkg/plugins/resources/yaml/main.go --------- Signed-off-by: Olblak <me@olblak.com> Co-authored-by: Olivier Vernin <olivier@vernin.me> Co-authored-by: Olblak <me@olblak.com>

lemeurherve changed the title ~~[wip] feat(file): implement spec.files for kind: file (first part: targets block)~~ [wip] feat(file): implement spec.files for kind: file Jan 30, 2022

dduportal reviewed Jan 30, 2022

View reviewed changes

Comment thread pkg/plugins/file/main_test.go Outdated

lemeurherve changed the title ~~[wip] feat(file): implement spec.files for kind: file~~ feat(file): implement spec.files for kind: file Jan 30, 2022

lemeurherve marked this pull request as ready for review January 31, 2022 00:41

lemeurherve requested review from dduportal and olblak January 31, 2022 00:41

dduportal reviewed Jan 31, 2022

View reviewed changes

Comment thread .golangci.yml

lemeurherve marked this pull request as draft January 31, 2022 10:49

lemeurherve commented Jan 31, 2022

View reviewed changes

Comment thread pkg/core/text/mocks.go Outdated

lemeurherveCB added 19 commits March 23, 2022 23:30

wip: implement 'spec.files' for 'kind: file' (first part: 'targets')

60aeb44

fix: correct example and filename

0ff4f8c

wip: fix

f9bc3b9

fix: spec.file & spec.files mutually exclusives; simplify validation

afaafb0

fix: no more imbricated if...else

f3cbee5

fix: if

d97ae8d

fix: duplicated LICENSE example in the correct folder

9164809

fix: sources

8c0ba9e

fix: conditions

47d92b5

chore: remove File.CurrentContent

6b52be4

chore: clean comments

0f53bb8

fix: no more CurrentContent

fe1d443

fix: read complete file only if spec.line == 0

b8a908e

fix: first stab at tests (?)

e573539

fix/force: main_test

0be4ed0

something: no files for main_test.validate

855360c

fix: validate on the Spec instead of the File

c1b0c1e

fix: main_test.Test_Validate complete

2a126dd

fix: no need to test for 'file' & 'files' or no 'file' nor 'files' as…

88ba98d

… it's already checked at the spec validation

fix: correct files complete path for source

39a9b17

Merge pull request #18 from lemeurherve/is-435-without-lines

6793444

lemeurherveCB and others added 2 commits April 5, 2022 20:48

fixup! chore: remove lines from text retriever mock

25172f0

Merge branch 'main' into is-435

f1224f9

Merge branch 'main' into is-435

f380e0a

olblak reviewed Apr 6, 2022

View reviewed changes

Comment thread pkg/plugins/resources/dockerfile/condition_test.go Outdated

fix test case

1b780c2

Co-authored-by: Olivier Vernin <olivier@vernin.me>

olblak approved these changes Apr 6, 2022

View reviewed changes

dduportal approved these changes Apr 6, 2022

View reviewed changes

dduportal merged commit aeacb2f into updatecli:main Apr 6, 2022

lemeurherve pushed a commit to lemeurherve/updatecli that referenced this pull request Apr 11, 2022

feat(yaml): implement spec.files for kind: yaml

3c0f548

Follow-up of updatecli#490 Ref updatecli#435

lemeurherve pushed a commit to lemeurherve/updatecli that referenced this pull request Apr 14, 2022

feat(yaml): implement spec.files for kind: yaml

45c99d3

Follow-up of updatecli#490 Ref updatecli#435

lemeurherve pushed a commit to lemeurherve/updatecli that referenced this pull request Apr 14, 2022

feat(yaml): implement spec.files for kind: yaml

96c6acd

Follow-up of updatecli#490 Ref updatecli#435

lemeurherve mentioned this pull request Apr 14, 2022

Add spec.files for Condition and Target of kind yaml #650

Merged

lemeurherve pushed a commit to lemeurherve/updatecli that referenced this pull request Sep 10, 2022

feat(yaml): implement spec.files for kind: yaml

83cd57d

Follow-up of updatecli#490 Ref updatecli#435

Uh oh!

Conversation

lemeurherve commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ability to specify more than one file for kind: file

Additional validations

Test

Additional Information

Potential improvements

Uh oh!

lemeurherve commented Jan 30, 2022

Uh oh!

Uh oh!

lemeurherve commented Jan 31, 2022

Uh oh!

Uh oh!

Uh oh!

olblak commented Mar 30, 2022

Uh oh!

olblak commented Mar 30, 2022

Uh oh!

olblak commented Mar 30, 2022

Uh oh!

lemeurherve commented Mar 31, 2022

Uh oh!

lemeurherve commented Apr 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

olblak commented Apr 4, 2022

Uh oh!

olblak commented Apr 5, 2022

Uh oh!

lemeurherve commented Apr 5, 2022

Uh oh!

Uh oh!

lemeurherve commented Apr 6, 2022

Uh oh!

olblak commented Apr 6, 2022

Uh oh!

lemeurherve commented Apr 6, 2022

Uh oh!

olblak commented Apr 6, 2022

Uh oh!

olblak commented Apr 6, 2022

Uh oh!

olblak commented Apr 6, 2022

Uh oh!

dduportal left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lemeurherve commented Jan 26, 2022 •

edited

Loading

Ability to specify more than one file for `kind: file`

lemeurherve commented Apr 2, 2022 •

edited

Loading