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

daemon: allowing adding external http handlers #265

Merged
merged 10 commits into from
Aug 4, 2023

Conversation

paul-rodriguez
Copy link
Contributor

@paul-rodriguez paul-rodriguez commented Jul 28, 2023

This PR turns the daemon API into an exported variable.

External tools depending on pebble may append to the variable to extend the daemon API, see snippet below.

type fakeHandler struct {
	cmd        *daemon.Command
	lastMethod string
}

func (h *fakeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	h.lastMethod = r.Method
	fmt.Println("Fake handler called")
}

func main() {

	var handler fakeHandler

	getCallback := func(c *daemon.Command, r *http.Request, s *daemon.UserState) daemon.Response {
		handler.cmd = c
		fmt.Println("getCallback called")
		return &handler
	}

	command := daemon.Command{
		Path:    "/v1/addedendpoint",
		GuestOK: true,
		GET:     getCallback,
	}
	
	// Here's the relevant line:
	daemon.API = append(daemon.API, &command)

	if err := cli.Run(); err != nil {
		fmt.Fprintf(cli.Stderr, "error: %v", err)
		os.Exit(1)
	}
	
	// Executable now handles requests for "/v1/addedendpoint"
}

@paul-rodriguez paul-rodriguez changed the title Extensible daemon daemon: allowing registering external http handlers Jul 28, 2023
internals/daemon/api.go Outdated Show resolved Hide resolved
internals/daemon/daemon.go Outdated Show resolved Hide resolved
@flotter flotter self-requested a review August 2, 2023 07:52
@paul-rodriguez paul-rodriguez changed the title daemon: allowing registering external http handlers daemon: allowing adding external http handlers Aug 2, 2023
internals/daemon/daemon_test.go Outdated Show resolved Hide resolved
@paul-rodriguez paul-rodriguez marked this pull request as ready for review August 2, 2023 08:33
Copy link
Contributor

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Approved, but note we should change Api -> API

internals/daemon/api.go Outdated Show resolved Hide resolved
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

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

Looks good. Just one detail needs to be fixed on the test.

internals/daemon/daemon_test.go Show resolved Hide resolved
@flotter
Copy link
Contributor

flotter commented Aug 3, 2023

Gustavo reviewed & approved, pending comment fix and conflict resolved

@flotter flotter added the High Priority Look at me first label Aug 3, 2023
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

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

Sorry, but still has issues. :(

internals/daemon/daemon_test.go Outdated Show resolved Hide resolved
@jnsgruk jnsgruk requested a review from niemeyer August 4, 2023 08:12
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

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

Thanks!

@niemeyer niemeyer merged commit 73aa51e into canonical:master Aug 4, 2023
13 checks passed
@paul-rodriguez paul-rodriguez deleted the extensible-daemon branch August 4, 2023 09:54
flotter added a commit to flotter/pebble that referenced this pull request Aug 31, 2023
The following PR provided Pebble derived projects to extend the supplied
daemon HTTP API handlers.

canonical#265

However, currently such a handler cannot access the daemon, even though we
added a daemon method to also expose the Overlord.

func v1PostDevice(
	*daemon.Command,
	req *http.Request,
	_ *daemon.UserState) daemon.Response {

	:

	// Cannot access c.d (daemon is private)
        ovld := c.d.Overlord()

	:
}

This PR adds c.Daemon() to allow access inside externally defined HTTP API
handlers.
niemeyer pushed a commit that referenced this pull request Aug 31, 2023
The following PR provided Pebble derived projects to extend the supplied
daemon HTTP API handlers.

#265

However, currently such a handler cannot access the daemon, even though we
added a daemon method to also expose the Overlord.

func v1PostDevice(
	*daemon.Command,
	req *http.Request,
	_ *daemon.UserState) daemon.Response {

	:

	// Cannot access c.d (daemon is private)
        ovld := c.d.Overlord()

	:
}

This PR adds c.Daemon() to allow access inside externally defined HTTP API
handlers.
@benhoyt benhoyt mentioned this pull request Sep 1, 2023
flotter added a commit that referenced this pull request Sep 29, 2023
#265 provides the ability for the daemon REST API to be extended by appending new handlers to the API list.

For example, the following new reboot command can be added now:

// requestReboot asks the daemon to gracefully shut down the system
// and issue a reboot.
func requestReboot(d *daemon.Daemon) daemon.Response {
        d.HandleRestart(restart.RestartSystem)
        result := deviceResult{}
        return daemon.SyncResponse(result)
}

Note that SyncResponse was already public, which allows the code above to work.

Following this patch, we can now reply with a suitable error response:

:
    switch(...) {
    case reboot:
        requestReboot(...)
    default:    
        return daemon.ErrorResponse(http.StatusBadRequest,"invalid media type %q", mediaType)
    {
:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority Look at me first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants