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

feat: Supports the validate-false option at an app level. Closes #1063 #2542

Merged
merged 19 commits into from
Feb 10, 2020

Conversation

alexec
Copy link
Contributor

@alexec alexec commented Oct 22, 2019

Checklist:

Closes #1063

Reviewer checklist:

  • Happy with the schema changes?
  • Happy with using ! to remove flag? Similar to how other tools do this.
  • @alexmt can you please look at the changes around syncPolicy.automated in the UI?

@codecov
Copy link

codecov bot commented Oct 22, 2019

Codecov Report

Merging #2542 into master will decrease coverage by 0.88%.
The diff coverage is 35.2%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2542      +/-   ##
==========================================
- Coverage   39.53%   38.65%   -0.89%     
==========================================
  Files          40      168     +128     
  Lines        1171    18248   +17077     
  Branches      203      237      +34     
==========================================
+ Hits          463     7053    +6590     
- Misses        706    10321    +9615     
- Partials        2      874     +872
Impacted Files Coverage Δ
ui/src/app/shared/components/page.tsx 39.28% <ø> (+1.78%) ⬆️
.../src/app/applications/components/resource-label.ts 100% <ø> (ø)
server/account/account.go 43.75% <ø> (ø)
.../app/shared/components/progress/progress-popup.tsx 90.9% <ø> (ø)
util/argo/audit_logger.go 0% <ø> (ø)
...hared/components/editable-panel/editable-panel.tsx 22% <ø> (-0.45%) ⬇️
util/clusterauth/clusterauth.go 21.64% <ø> (ø)
ui/src/app/applications/components/resources.ts 100% <ø> (ø)
reposerver/repository/repository.go 59.89% <ø> (ø)
.../app/shared/components/yaml-editor/yaml-editor.tsx 32.25% <ø> (ø) ⬆️
... and 181 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84f3b81...2021227. Read the comment docs.

@alexec
Copy link
Contributor Author

alexec commented Oct 23, 2019

@yann-soubeyrand can you take a look at this solution please?

@yann-soubeyrand
Copy link
Contributor

Thanks a lot @alexec! I'm wondering if renaming the option noValidate to validate and defaulting it to true wouldn't clarify things (it would directly map to kubectl CLI option --validate=false)? Apart from that, this looks good to me and will totally address my needs ;-)

@alexec
Copy link
Contributor Author

alexec commented Oct 23, 2019

Thank you @yann-soubeyrand . @alexmt thoughts?

@alexec
Copy link
Contributor Author

alexec commented Oct 23, 2019

@yann-soubeyrand, @alexmt agrees.

…r.go,controller/sync.go,controller/sync_test.go,docs/operator-manual/application.yaml,manifests/crds/application-crd.yaml,manifests/ha/install.yaml,manifests/ha/namespace-install.yaml,manifests/install.yaml,manifests/namespace-install.yaml,pkg/apis/application/v1alpha1/generated.pb.go,pkg/apis/application/v1alpha1/generated.proto,pkg/apis/application/v1alpha1/openapi_generated.go,pkg/apis/application/v1alpha1/types.go,pkg/apis/application/v1alpha1/types_test.go,pkg/apis/application/v1alpha1/zz_generated.deepcopy.go,server/application/application.go,
@alexec alexec changed the title Adds noValidate option. Closes #1063 Supports the validate-false option at an app level. Closes #1063 Oct 23, 2019
@alexec alexec marked this pull request as ready for review October 25, 2019 20:15
pkg/apis/application/v1alpha1/types.go Outdated Show resolved Hide resolved
….go,cmd/argocd/commands/app_test.go,controller/appcontroller.go,controller/sync.go,controller/sync_test.go,manifests/crds/application-crd.yaml,manifests/ha/install.yaml,manifests/ha/namespace-install.yaml,manifests/install.yaml,manifests/namespace-install.yaml,pkg/apis/application/v1alpha1/generated.pb.go,pkg/apis/application/v1alpha1/generated.proto,pkg/apis/application/v1alpha1/openapi_generated.go,pkg/apis/application/v1alpha1/types.go,pkg/apis/application/v1alpha1/types_test.go,pkg/apis/application/v1alpha1/zz_generated.deepcopy.go,server/application/application.go,
…ion-crd.yaml,manifests/ha/install.yaml,manifests/ha/namespace-install.yaml,manifests/install.yaml,manifests/namespace-install.yaml,pkg/apis/application/v1alpha1/generated.pb.go,pkg/apis/application/v1alpha1/generated.proto,pkg/apis/application/v1alpha1/openapi_generated.go,
@alexec alexec dismissed jessesuen’s stale review November 1, 2019 16:30

changes implemented

@alexec alexec requested a review from jessesuen November 1, 2019 16:30
@alexec
Copy link
Contributor Author

alexec commented Nov 1, 2019

Changes since last review:

  • Change schema to add syncPolicy.SyncOptions []string, as discussed with @jessesuen .
  • Adds support to CLI to add an app with sync-options.
  • Adds support to UI to add and app, or update an app's sync options.

@alexec
Copy link
Contributor Author

alexec commented Nov 1, 2019

CLI examples:

argocd app create guestbook --repo https://github.com/argoproj/argocd-example-apps.git --path guestbook --dest-namespace default --dest-server https://kubernetes.default.svc --directory-recurse --sync-option Validate=false

Unset flag:

$ argocd app set guestbook --sync-option '!Validate=false'

Set flag:

$ argocd app set guestbook --sync-option 'Validate=false'

@alexec alexec requested a review from alexmt November 1, 2019 18:21
…ion-create-panel/application-create-panel.tsx,ui/src/app/applications/components/application-summary/application-summary.tsx,
@alexec alexec added enhancement New feature or request L labels Nov 22, 2019
@alexec alexec added this to the v1.4 milestone Nov 27, 2019
@alexec alexec changed the title Supports the validate-false option at an app level. Closes #1063 feat: Supports the validate-false option at an app level. Closes #1063 Dec 17, 2019
@yann-soubeyrand
Copy link
Contributor

Hi, what's the status of this PR? What can we do to help it being merged?

@alexmt alexmt modified the milestones: v1.4, v1.5 Jan 23, 2020
@LeonardAukea
Copy link

It seems that we are dependent on this PR to be merged in order to deploy cert-manager with argo without using multiple hacks that fall short. What is the status of this and is there anything we can do to speed up the process?

@alexec
Copy link
Contributor Author

alexec commented Jan 29, 2020

@jessesuen @alexmt do you want to progress this in v1.5 please? I can find some time to fix the merges, but that would only make sense if you have time to review the PR. Can I please ask if you can review the PR? Thank you.

@alexec alexec closed this Feb 9, 2020
@alexmt
Copy link
Collaborator

alexmt commented Feb 10, 2020

@alexec I'm working on merging master changes and preparing to merge this PR

@alexmt alexmt reopened this Feb 10, 2020
@alexec
Copy link
Contributor Author

alexec commented Feb 10, 2020

ok - let me know if you need any assist

@alexmt
Copy link
Collaborator

alexmt commented Feb 10, 2020

@alexec , I've changed the UI for sync options. Instead of free form test implemented checkboxes, so user don't have to learn syntax of supported options:

image

image

does it looks good to you?

@alexmt alexmt merged commit ebc0481 into argoproj:master Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to kubectl apply objects using --validate=false
6 participants