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: make corehttp a reusable component #9070

Merged
merged 5 commits into from
Aug 17, 2022
Merged

feat: make corehttp a reusable component #9070

merged 5 commits into from
Aug 17, 2022

Conversation

iand
Copy link
Contributor

@iand iand commented Jun 30, 2022

This makes the minimal changes needed to allow the corehttp package to be imported and reused by external applications.

Fixes #8524

It adds a new interface that defines the minimal set of methods needed to run corehttp as a gateway and is a non-breaking change.

As an example of use, where s.Gateway implements the four methods from the new interface

func (s *Server) registerHandlers() error {
	s.mux = http.NewServeMux()

	headers := make(map[string][]string)
	corehttp.AddAccessControlHeaders(headers)

	cfg := corehttp.GatewayConfig{
		Writable: false,
		Headers:  headers,
	}

	gwHandler := corehttp.NewGatewayHandler(cfg, s.Gateway)
	s.mux.Handle("/ipfs/", gwHandler)
	s.mux.Handle("/ipns/", gwHandler)

	return nil
}

(From https://github.com/iand/meridian/blob/006cf437183d07005e7b6254cb5a551eb64b102a/server.go#L77:L93)

@iand iand requested a review from lidel as a code owner June 30, 2022 15:18
@aschmahmann
Copy link
Contributor

aschmahmann commented Jun 30, 2022

Looks great! Is a minimal change. Hopefully we can shrink that interface and get it even more minimal in the future.


There are some failing sharness tests here related to the gateway and seem related to your changes.

The failing tests are in test/sharness/t0117-gateway-block.sh.

expecting success: 
  grep "< Access-Control-Allow-Headers: Range" curl_output &&
  grep "< Access-Control-Allow-Headers: X-Custom1" curl_output &&
  grep "< Access-Control-Expose-Headers: Content-Range" curl_output &&
  grep "< Access-Control-Expose-Headers: Content-Length" curl_output &&
  grep "< Access-Control-Expose-Headers: X-Ipfs-Path" curl_output &&
  grep "< Access-Control-Expose-Headers: X-Ipfs-Roots" curl_output &&
  grep "< Access-Control-Expose-Headers: X-Custom2" curl_output

< Access-Control-Allow-Headers: Range

not ok 23 - Access-Control-Allow-Headers extends
#	
#	  grep "< Access-Control-Allow-Headers: Range" curl_output &&
#	  grep "< Access-Control-Allow-Headers: X-Custom1" curl_output &&
#	  grep "< Access-Control-Expose-Headers: Content-Range" curl_output &&
#	  grep "< Access-Control-Expose-Headers: Content-Length" curl_output &&
#	  grep "< Access-Control-Expose-Headers: X-Ipfs-Path" curl_output &&
#	  grep "< Access-Control-Expose-Headers: X-Ipfs-Roots" curl_output &&
#	  grep "< Access-Control-Expose-Headers: X-Custom2" curl_output
#	

expecting success: 
  grep "< Access-Control-Allow-Origin: localhost" curl_output

not ok 24 - Access-Control-Allow-Origin replaces
#	
#	  grep "< Access-Control-Allow-Origin: localhost" curl_output
#	

@iand
Copy link
Contributor Author

iand commented Jul 1, 2022

@aschmahmann fixed header issue, PTAL

@iand
Copy link
Contributor Author

iand commented Jul 21, 2022

Updated to incorporate changes from #9082

go.mod Outdated Show resolved Hide resolved
@BigLep BigLep mentioned this pull request Aug 12, 2022
72 tasks
@BigLep
Copy link
Contributor

BigLep commented Aug 15, 2022

@iand : we are hoping to land this for the 0.15 RC that we're cutting on Wednesday, 2022-08-17. Are you able to finish responding to comments so we can land this?

@iand
Copy link
Contributor Author

iand commented Aug 16, 2022

@iand : we are hoping to land this for the 0.15 RC that we're cutting on Wednesday, 2022-08-17. Are you able to finish responding to comments so we can land this?

Done.

@lidel lidel self-assigned this Aug 16, 2022
@lidel lidel mentioned this pull request Aug 17, 2022
2 tasks
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you, @iand.

This is a good step towards #8524 (comment), and the changed surface is small enough to be safe to be included in 0.15-rc1

@lidel lidel merged commit 924ab06 into ipfs:master Aug 17, 2022
hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
feat: make corehttp a reusable component

This commit was moved from ipfs/kubo@924ab06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Gateway Extraction to go-libipfs/gateway
4 participants