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: add pages command #9811

Closed
wants to merge 4 commits into from
Closed

feat: add pages command #9811

wants to merge 4 commits into from

Conversation

sapk
Copy link
Member

@sapk sapk commented Jan 16, 2020

This new command "proxy" the raw API (should be under another domain for security) with mime-type and more user friendly url. This command need a running gitea

How to use it :

./gitea pages -r "https://try.gitea.io" -b master

Things to improve :

  • Cache
  • HTTP header
  • Cli command
  • Compression
  • Token
  • TLS
  • Tests
  • Security
    (in short everything 😄)

Issues related: #9305 #3834 #1721 #8152

@sapk sapk added type/feature Completely new functionality. Can only be merged if feature freeze is not active. pr/wip This PR is not ready for review labels Jan 16, 2020
@codecov-io
Copy link

codecov-io commented Jan 16, 2020

Codecov Report

Merging #9811 into master will decrease coverage by 0.04%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9811      +/-   ##
==========================================
- Coverage   42.22%   42.17%   -0.05%     
==========================================
  Files         603      604       +1     
  Lines       78881    78956      +75     
==========================================
- Hits        33309    33303       -6     
- Misses      41495    41576      +81     
  Partials     4077     4077
Impacted Files Coverage Δ
cmd/pages.go 0% <0%> (ø)
services/pull/patch.go 64.77% <0%> (-3.15%) ⬇️
services/pull/temp_repo.go 31.62% <0%> (-2.57%) ⬇️
modules/log/event.go 64.61% <0%> (-1.03%) ⬇️
models/repo.go 46.74% <0%> (-0.13%) ⬇️
models/pull.go 42.59% <0%> (+0.6%) ⬆️
services/pull/check.go 56.64% <0%> (+2.09%) ⬆️

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 d3468ed...429638b. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 16, 2020
@lunny
Copy link
Member

lunny commented Jan 17, 2020

If we didn't give a -r, we could read the config and know the domain. That really resolve the security problem but satisfy many users' requirements.

@lafriks
Copy link
Member

lafriks commented Jan 17, 2020

Can't we detect request domain name in go code if behind reverse proxy?

@lafriks
Copy link
Member

lafriks commented Jan 17, 2020

Or we could allow setting in app.ini to start listener on other port/socket for this

@lunny
Copy link
Member

lunny commented Jan 17, 2020

Further more, I think we could implement the pages.
We can define a table named repo_pages which has columns repo_id, branch_name, page_name. The page_name should be unique.
User could indicate page name and branch name on the repository settings and gitea pages command will monitor the table pages(or via webhooks) to render.
And there are two modes, one is for domain mode, the command will render only domain requests and will follow the page1.domain.com to render page1.
Another mode is for ip visit, all pages will be visited via [ip]/page1.

@sapk
Copy link
Member Author

sapk commented Jan 17, 2020

I open early this PR, to discuss about it because I also hesitate on various design. First I was thinking that it was better to keep it separate (even from config file and maybe a separate binary) to be like a separate micro service that could scale. But I also see benefit to keep it inside gitea and directly load from config file and even integrate it in gitea (listening on another port but we have to warn/check domain limits).

For domain detection behind proxy it is possible. The proxy just have to set some header but that near standard.

@lafriks
Copy link
Member

lafriks commented Jan 17, 2020

If this service is built into gitea directly right checks and more features could be created more easier than it would be if they are separate

@stale
Copy link

stale bot commented May 6, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label May 6, 2020
@stale
Copy link

stale bot commented Jul 5, 2020

This pull request has been automatically closed because of inactivity. You can re-open it if needed.

@stale stale bot closed this Jul 5, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/stale lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. pr/wip This PR is not ready for review type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants