Cc 7145 hcp link status api #20330

valeriia-ruban · 2024-01-24T08:54:13Z

Description

https://hashicorp.atlassian.net/browse/CC-7145
Due to the API specific, it doesn't fall well to the data loading approach which is used for regular records.
So I did similar to autopilot loading, which as well not really fall under record architecture(list + crud).

for 404 link/global and for STATE_FALSE repository maps the isLinking value to false. For STATE_TRUE - to true. For other error codes, repo return null for the result object (to make it possible to understand when some other system/auth error raised)

Testing & Reproduction steps

add Linked is: {{hcpLink.data.isLinked}} in a next line
using consul-setup repo and its README
Build consul
login with ACLS consul acl bootstrap, using secretId
check the label

It is also possible to check it with yarn start. Mock data set to STATE_TRUE by default

link/global could be added with triggering from browser console

fetch("https://localhost:8501/api/hcp/v2/link/global", {
  "headers": {
    "accept": "*/*",
    "accept-language": "en-US,en;q=0.9",
    "content-type": "application/json; charset=utf-8",
    "sec-ch-ua": "\"Not_A Brand\";v=\"8\", \"Chromium\";v=\"120\"",
    "sec-ch-ua-mobile": "?0",
    "sec-ch-ua-platform": "\"macOS\"",
    "sec-fetch-dest": "empty",
    "sec-fetch-mode": "cors",
    "sec-fetch-site": "same-origin",
    "x-consul-token": "<PAST HERE Secret ID>",
    "x-requested-with": "XMLHttpRequest"
  },
  "referrer": "https://localhost:8501/ui/dc1/services",
  "referrerPolicy": "strict-origin-when-cross-origin",
  "body": JSON.stringify(
      { data: {
          client_id: "Sq2HHIo5TjBvasTXTJW8L9yRzwjEaERs",
          resource_id: "organization/7309cdc4-d05a-4d84-af5d-40bf5adbb424/project/e38aa34e-1120-43f1-9092-c529e8595e3f/hashicorp.consul.global-network-manager.cluster/eeeeeee",
          client_secret: "elg9GrWkz6KjWBR6tgQkrix41LKcgHlUHxwArQuUZk46JFBq6KXsRhuEt8T5qyC_"
      }
      }
  ),
  "method": "PUT",
  "mode": "cors",
  "credentials": "include"
});

Links

PR Checklist

updated test coverage
external facing docs updated
appropriate backport labels added
not a security concern

chris-hut

Approved! Very nice work, I think some tests are incoming - but love seeing this up and will unblock us in some other PRs!

chris-hut · 2024-01-24T18:15:45Z

ui/packages/consul-ui/app/services/repository/hcp-link.js

+          meta: {
+            version: 2,
+            uri: uri,
+          },


Is this just "magic" ™️ to make the repo service happy?

chris-hut · 2024-01-24T18:17:06Z

ui/packages/consul-ui/app/services/repository/hcp-link.js

+            isLinked: (body.status['consul.io/hcp/link']['conditions'] || []).some(
+              (condition) => condition.type === 'linked' && condition.state === 'STATE_TRUE'


Was going to ask why we have the array methods here, but the API response does have conditions to be an array, I wonder if we'd ever see multiple contradicting conditions?

I hope not - feels like something we should handle in the backend

yea, it is how the backend responds as of now. I've just decided to check that condition is linked, just in case we will have some other stuff added in future.

But I totally agree, that as of now we don't assume to have something else to be returned from the endpoint

chris-hut · 2024-01-24T18:18:58Z

ui/packages/consul-ui/app/services/repository/hcp-link.js

+      // set linked to false if the global link is not found
+      if (e.statusCode === 404) {
+        result = Promise.resolve({ isLinked: false });


Love the comment!

chris-hut · 2024-01-24T18:26:48Z

ui/packages/consul-ui/app/services/repository/hcp-link.js

+      if (e.statusCode === 404) {
+        result = Promise.resolve({ isLinked: false });
+      } else {
+        result = Promise.resolve(null);


So if we get an error (auth related or anything really), null would probably act as an isLinked: false as in code if we tried to do a

{{#if hcpLlink.isLinked}} Say cluster is linked {{else}} <DisplayBanner /> {{/if}}

Thinking aloud, is that what we'd want to do?

If we fail to fetch this result because our ACL token doesn't have enough permissions - this would return notLinked - which is okay - since we have code ensuring we only display banner if user has permission to link

If we fail for other reasons - I think we should probably check that we don't have a definitive result to the is this cluster linked in the hcp-link-statusservice, which is what you were saying with repo return null for the result object (to make it possible to understand when some other system/auth error raised) I think

So I think this is fine/great! However we hook this up to the hcp-link-status, we'll probably have to handle that there!

yea, the idea was to add some state, where we could see that hcpLink could not actually be observed as linked or not. there are other ways to do it, for sure, like to set {isLinked: null} or so.

So I open to discuss it

chris-hut · 2024-01-24T18:28:40Z

ui/packages/consul-ui/app/templates/application.hbs

                as |dc|
              >
                {{#if dc.data}}
-                  <HashicorpConsul
+                  <DataSource @src={{uri '/${partition}/*/${dc}/hcp-link' (hash dc=dc.Name partition=partition name=dc.Name) }} @open={{true}} as |hcpLink|>


Bit curious about this - Does this mean we'll fetch this upon application load? And this data will be accessible as the hcpLink variable in this template?

Would we still need/want this if we're only requesting this inside of the hcpLinkStatus service?

good question. when application loading, we calling the service. but we have to call it on login as well
due to <HashicorpConsul> component has the tag structure and we need the data in sidenav and in main tags, I've put the call to application and make it open. it means that on login we will have it reloaded, same as on route change.

It is questionable though, when we need to reload it. let's say if we are linking in HCP, and does not reload page in core, I think we still should get the information if it is linked

actually it depends on the cost of calling the service, which in our case seems to be not too bad, because it uses local consul... what do you think?

…nsul into CC-7145-hcp-link-status-api

…ervice

This reverts commit 049ca10.

* Revert "feat: add alert to link to hcp modal to ask a user refresh a page; up… (#20682)" This reverts commit dd833d9. * Revert "chor: change cluster name param to have datacenter.name as default value (#20644)" This reverts commit 8425cd0. * Revert "chor: adds informative error message when acls disabled and read-only… (#20600)" This reverts commit 9d712cc. * Revert "Cc 7147 link to hcp modal (#20474)" This reverts commit 8c05e57. * Revert "Add nav bar item to show HCP link status and encourage folks to link (#20370)" This reverts commit 22e6ce0. * Revert "Cc 7145 hcp link status api (#20330)" This reverts commit 049ca10. * Revert "💜 Cc 7187/purple banner for linking existing clusters (#20275)" This reverts commit 5119667.

valeriia-ruban added 5 commits January 22, 2024 22:17

feat: add api call to hcp/link endpoint

a0de3be

updated

98bee0a

updated

50b93a2

update approach to get the linking status

6baa6fc

updated application template

2fbc210

valeriia-ruban requested a review from chris-hut January 24, 2024 08:54

github-actions bot added the theme/ui Anything related to the UI label Jan 24, 2024

chris-hut approved these changes Jan 24, 2024

View reviewed changes

valeriia-ruban added 6 commits January 25, 2024 12:29

feat: add api call to hcp/link endpoint

c7083b2

updated

967f00f

updated

2b34642

update approach to get the linking status

7042752

updated application template

664db73

update purple banner links

c4a15ff

valeriia-ruban force-pushed the CC-7145-hcp-link-status-api branch from 2fbc210 to c4a15ff Compare January 25, 2024 21:00

chris-hut added 2 commits January 25, 2024 13:03

Merge branch 'CC-7145-hcp-link-status-api' of github.com:hashicorp/co…

aac0270

…nsul into CC-7145-hcp-link-status-api

Hook up the linked check to the purple banner

471dbb2

chris-hut added pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry labels Jan 25, 2024

valeriia-ruban and others added 3 commits January 25, 2024 21:17

fixed lint issue

5903715

Updated tests for new link status API calls as args instead of from s…

098329a

…ervice

Merge branch 'main' into CC-7145-hcp-link-status-api

b6f3fce

chris-hut merged commit 049ca10 into main Jan 26, 2024
59 checks passed

chris-hut deleted the CC-7145-hcp-link-status-api branch January 26, 2024 17:57

chris-hut added a commit that referenced this pull request Mar 11, 2024

Revert "Cc 7145 hcp link status api (#20330)"

731f276

This reverts commit 049ca10.

chris-hut mentioned this pull request Mar 11, 2024

Revert link existing but better 🪦 #20830

Merged

dhiaayachi mentioned this pull request Sep 26, 2024

Revert link existing but better 🪦 dhiaayachi/consul#785

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cc 7145 hcp link status api #20330

Cc 7145 hcp link status api #20330

valeriia-ruban commented Jan 24, 2024 •

edited

Loading

chris-hut left a comment

chris-hut Jan 24, 2024

valeriia-ruban Jan 24, 2024

chris-hut Jan 24, 2024

valeriia-ruban Jan 24, 2024

chris-hut Jan 24, 2024

chris-hut Jan 24, 2024

valeriia-ruban Jan 24, 2024

chris-hut Jan 24, 2024

valeriia-ruban Jan 24, 2024

		isLinked: (body.status['consul.io/hcp/link']['conditions'] \|\| []).some(
		(condition) => condition.type === 'linked' && condition.state === 'STATE_TRUE'

Cc 7145 hcp link status api #20330

Cc 7145 hcp link status api #20330

Conversation

valeriia-ruban commented Jan 24, 2024 • edited Loading

Description

Testing & Reproduction steps

Links

PR Checklist

chris-hut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valeriia-ruban commented Jan 24, 2024 •

edited

Loading