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.

Sign up for GitHub

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

Jump to bottom

Allow loading file from http #2167

Merged
merged 2 commits into from
Mar 6, 2020
Merged

Allow loading file from http #2167

merged 2 commits into from
Mar 6, 2020

Conversation

yujunz
Copy link
Member

@yujunz yujunz commented Feb 1, 2020 •
edited
Loading

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 1, 2020
@yujunz yujunz force-pushed the loader/http branch 3 times, most recently from a8df26c to 609c910 Compare February 1, 2020 03:02
@yujunz yujunz mentioned this pull request Feb 1, 2020
Closed
@yujunz yujunz changed the title Allow loading from http Allow loading file from http Feb 1, 2020
@yujunz yujunz force-pushed the loader/http branch 2 times, most recently from 09732d7 to f80c574 Compare February 1, 2020 05:35
@alex1989hu alex1989hu mentioned this pull request Feb 18, 2020
Merged
@pwittrock pwittrock added this to In progress in kanban via automation Feb 24, 2020
pwittrock
pwittrock reviewed Feb 24, 2020
View reviewed changes
api/loader/fileloader.go Outdated Show resolved Hide resolved
@pwittrock pwittrock moved this from In progress to Review in progress in kanban Feb 24, 2020
@pwittrock pwittrock added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 24, 2020
yujunz
yujunz commented Feb 25, 2020
View reviewed changes
Copy link
Member Author

@yujunz yujunz left a comment

Choose a reason for hiding this comment

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

Replied inline.

@pwittrock
Copy link
Contributor

@monopole -- FYI - this looks like something that would pretty clearly be nice to have

@yujunz yujunz force-pushed the loader/http branch 2 times, most recently from d50634c to 0d900fa Compare February 28, 2020 12:06
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 28, 2020
@yujunz
Copy link
Member Author

yujunz commented Feb 29, 2020

Made minor change to scheme check and added unit test @pwittrock

@pwittrock
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 6, 2020
@pwittrock
Copy link
Contributor

Thanks @yujunz !

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pwittrock, yujunz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 6, 2020
@pwittrock
Copy link
Contributor

/test kustomize-presubmit-master

@pwittrock
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 6, 2020
@k8s-ci-robot k8s-ci-robot merged commit 300dd10 into kubernetes-sigs:master Mar 6, 2020
kanban automation moved this from Review in progress to Done Mar 6, 2020
@yujunz yujunz deleted the loader/http branch March 7, 2020 01:39
pohly
pohly reviewed Mar 7, 2020
View reviewed changes
examples/loadHttp.md
```

Kustomize will try loading resource as a file either from local or http. If it
fails, try to load it as a directory or git repository.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "load it as a directory or git repository" mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

Example of load as a directory or git repository:

resources:
- ../base
- github.com/kustless/kustomize-examples

It is not possible to tell if it is a file before trying to load.

Copy link
Contributor

Choose a reason for hiding this comment

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

So that example is now valid? The documentation only mentions loading via http[s]:// urls.

Copy link
Member Author

Choose a reason for hiding this comment

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

The example should be valid at 300dd10 but might be broken on 0b1ad03 , see #2264 .

github.com/kustless/kustomize-examples is supported by the original git cloner. http loader is for single yaml file only as given in the example. It does not apply for git repository or http archive. When failed to load file, it will fallback to original directory loader and git cloner.

#2169 try to solve the remote get problem with go-getter but it was merged by accident before completely tested...

@johananl johananl mentioned this pull request Mar 30, 2020
Closed
@yellowmegaman
Copy link

yellowmegaman commented Apr 29, 2020 •
edited
Loading

So it's not working anymore? Way too hard to find out anything with so many discussions

@leoskyrocker
Copy link

leoskyrocker commented Apr 29, 2020 •
edited
Loading

hey @yellowmegaman, you can always check whether a merged commit is contained in a tagged version by navigating to the commit itself like here.

As you can see, as of writing, there is no kustomize version that contains this commit, which means that this change is not really in any release yet, so it's not going to work yet.

Screenshot 2020-04-29 at 11 13 02 PM

@mhaddon
Copy link

mhaddon commented May 7, 2020

will this work for configuations blocks?

@rverma-jm rverma-jm mentioned this pull request May 25, 2020
Closed
@stephen37 stephen37 mentioned this pull request Sep 7, 2021
Merged
@guicassolato guicassolato mentioned this pull request Feb 21, 2022
Merged
@annasong20 annasong20 mentioned this pull request May 6, 2022
Merged
@joebowbeer joebowbeer mentioned this pull request Mar 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pohly pohly pohly left review comments

@mengqiy mengqiy Awaiting requested review from mengqiy

@pwittrock pwittrock Awaiting requested review from pwittrock

Assignees

@pwittrock pwittrock

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
No open projects
kanban
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

resources: support remote files
7 participants
@yujunz @pwittrock @k8s-ci-robot @yellowmegaman @leoskyrocker @mhaddon @pohly