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

Begin adding origin key management #674

Merged
merged 1 commit into from
Jun 8, 2016
Merged

Begin adding origin key management #674

merged 1 commit into from
Jun 8, 2016

Conversation

smith
Copy link
Contributor

@smith smith commented Jun 8, 2016

  • Make it so you can go to /origins/x to view that origin (doesn't check
    for existence or permissions yet)
  • Very basic validation of key pasting (but you can't save yet)
  • Remove some logic around "default origin" that's not being used
  • Fix test runner to use correct loader

tumblr_mrxtj8qlf51s2yegdo1_400

* Make it so you can go to /origins/x to view that origin (doesn't check
  for existence or permissions yet)
* Very basic validation of key pasting (but you can't save yet)
* Remove some logic around "default origin" that's not being used
* Fix test runner to use correct loader

Signed-off-by: Nathan L Smith <[email protected]>
@thesentinels
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @ryankeairns and @fnichol to be potential reviewers

@reset
Copy link
Collaborator

reset commented Jun 8, 2016

gif-keyboard-16606231152096416321

@thesentinels r+

@thesentinels
Copy link
Contributor

📌 Commit 41d9e24 has been approved by reset

@thesentinels
Copy link
Contributor

⌛ Testing commit 41d9e24 with merge ef83681...

thesentinels pushed a commit that referenced this pull request Jun 8, 2016
* Make it so you can go to /origins/x to view that origin (doesn't check
  for existence or permissions yet)
* Very basic validation of key pasting (but you can't save yet)
* Remove some logic around "default origin" that's not being used
* Fix test runner to use correct loader

Signed-off-by: Nathan L Smith <[email protected]>

Pull request: #674
Approved by: reset
@thesentinels
Copy link
Contributor

☀️ Test successful - travis

@thesentinels thesentinels merged commit 41d9e24 into master Jun 8, 2016
@reset reset deleted the nls/origins branch June 8, 2016 19:30
jtimberman pushed a commit that referenced this pull request Jun 12, 2016
* Make it so you can go to /origins/x to view that origin (doesn't check
  for existence or permissions yet)
* Very basic validation of key pasting (but you can't save yet)
* Remove some logic around "default origin" that's not being used
* Fix test runner to use correct loader

Signed-off-by: Nathan L Smith <[email protected]>

Pull request: #674
Approved by: reset
stevendanna added a commit to stevendanna/habitat that referenced this pull request Oct 23, 2019
Previously, the gateway state had no health check information for a
given service until the first health-check. This resulted in a 404 for
the `/services/SERVICE_NAME/SERVICE_GROUP/health` endpoint. However,
according to the documentation, a 404 response code means the service
isn't loaded:

```
 /{name}/{group}/health:
     get:
         description: Health check status and output for the given service group
         responses:
             200:
                 description: Health Check - Ok / Warning
                 body:
                     application/json:
                         type: healthCheckOutput
             404:
                 description: Service not loaded
             500:
                 description: Health Check - Unknown
             503:
                 description: Health Check - Critical
```

This change avoids the problem by populating the gateway state with
Unknown when we start the health checks. As a result we now return 500
for the health endpoint of a loaded service until the first
health-check comes back, in accordance with the API documentation.

Fixes habitat-sh#674

Signed-off-by: Steven Danna <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants