Create UPGRADING.md #1607

hilagross · 2020-06-10T05:45:49Z

What does this PR do?

In this PR, we add upgrade instructions to this project. The instructions apply to docker-compose deployments of Conjur OSS. For helm-based upgrade instructions, users can see the Conjur helm chart upgrade guide.

In addition, this adds custom upgrade instructions for users who will be upgrading from pre-1.8 versions to 1.8+.

What ticket does this PR close?

Connected to #1584, #1528.

Checklists

Change log

The CHANGELOG has been updated, or
This PR does not include user-facing changes and doesn't require a CHANGELOG update

Test coverage

This PR includes new unit and integration tests to go with the code changes, or
The changes in this PR do not require tests

Documentation

This PR does not require updating any documentation, or
Docs (e.g. READMEs) were updated in this PR, and/or there is a follow-on issue to update docs

izgeri · 2020-06-10T21:29:19Z

UPGRADING.md

+
+## Specific steps
+
+**This step should be done  when upgrading from version 1.7.2 and below to a newer version**


Can we make this a markdown subsection? It would be convenient to use this going forward to note any additional version-specific instructions, and if the line is

### 1.8.0

Then we can link to it in our release notes and it's clear that the custom upgrade instructions apply to this specific version

Note that I am assuming the changes will land before the next tag, and that the tag will be a minor version bump.

@izgeri just to make sure I understood correctly, also the from version should be change to 1.8.0 or 1.7.2 is ok?

I am assuming that the FIPS changes will be included in the next tag, and that we'll bump the minor version since we're adding functionality in a backwards compatible manner (following semver - see here)

That would mean they'll be included in 1.8.0

that's for sure, what I meant should we say 'from 1.7.2 and below' or 'below version 1.8.0'?
what should be the correct term for this FYO?

We could just make it say Upgrading to 1.8.0

what will happen if a user will upgrade from 1.7.0 to 1.8.1 or even 1.9.0 in the future?
I think it should be clear the any upgrade from a version before 1.8.0 should run this step.
WDYT?
@alexkalish @izgeri

I think what you have is fine for now.

izgeri · 2020-06-16T17:49:38Z

@hilagross I'd like to have a technical review of this before we merge it. @diverdane and @sgnn7 have recently been through the upgrade process in great detail for the helm chart - the docker-compose upgrade flow is by comparison much simpler. I am checking with @alexkalish about whether we might be able to schedule a detailed technical review of this in the next week or so - I think the end result would really benefit from this.

alexkalish

@hilagross: Thank you for doing this! I made a ton of suggestions, mostly around wording and punctuation. I think that only my comment about the special step requires a manual change.

UPGRADING.md

alexkalish · 2020-06-19T18:02:18Z

UPGRADING.md

+
+## Specific steps
+
+**This step should be done  when upgrading from version 1.7.2 and below to a newer version**


I think what you have is fine for now.

InbalZilberman · 2020-07-14T18:23:26Z

@alexkalish @izgeri can you please take over this PR or close it and open another one?
Hila is not expected to return any time soon :)

izgeri · 2020-07-15T15:41:59Z

@alexkalish I will make your changes and try a quick sanity check before asking you to rereview

izgeri · 2020-07-15T21:20:42Z

@InbalZilberman @alexkalish I applied Alex's updates and then tried to run through the upgrade steps as documented using the quick start docker-compose with the Conjur image version specified (e.g. I modified this line to read image: cyberark/conjur:1.5.0, for example).

I was not able to get the original draft instructions to work, but a quick bit of research allowed me to tweak them to get them working for a standard upgrade from 1.5.0 to 1.7.4. Those updates are now in a separate commit on this branch.

Once I got those working, I tried the custom upgrade instructions for upgrading from 1.7.4 to 1.8.0. I was NOT SUCCESSFUL. Running bundle exec rake slosilo:migrate on the Conjur server container did not produce any log output, and when I attempted to interact with the API afterwards:

I was able to successfully authenticate, and exchange an API key for an access token

when I tried to use that access token on a secret retrieval request, I got an Invalid token response:

The retrieved value is: HTTP/1.1 401 Unauthorized
Server: nginx/1.13.6
Date: Wed, 15 Jul 2020 21:03:35 GMT
Content-Type: text/plain
Content-Length: 27
Connection: keep-alive
Cache-Control: no-cache
X-Request-Id: a27dc38f-0772-4fdc-b62b-a3181a641123
X-Runtime: 0.001891

Unauthorized: Invalid token

The Conjur logs show:

 Started POST "/authn/myConjurAccount/host%2FBotApp%2FmyDemoApp/authenticate" for 192.168.32.3 at 2020-07-15 21:03:33 +0000
[origin=192.168.32.3] [request_id=29c707d4-a7ff-4e58-a317-0071c60f6a30] [tid=36] Processing by AuthenticateController#authenticate as */*
[origin=192.168.32.3] [request_id=29c707d4-a7ff-4e58-a317-0071c60f6a30] [tid=36]   Parameters: {:controller=>"authenticate", :action=>"authenticate", :authenticator=>"authn", :account=>"myConjurAccount", :id=>"host/BotApp/myDemoApp"}
[origin=192.168.32.3] [request_id=29c707d4-a7ff-4e58-a317-0071c60f6a30] [tid=36] myConjurAccount:host:BotApp/myDemoApp successfully authenticated with authenticator authn service myConjurAccount:webservice:conjur/authn
[origin=192.168.32.3] [request_id=29c707d4-a7ff-4e58-a317-0071c60f6a30] [tid=36] Completed 200 OK in 17ms (Views: 0.2ms)
[origin=192.168.32.3] [request_id=a27dc38f-0772-4fdc-b62b-a3181a641123] [tid=34] Started GET "/secrets/myConjurAccount/variable/BotApp%2FsecretVar" for 192.168.32.3 at 2020-07-15 21:03:35 +0000

There is nothing after the Started GET line in the Conjur logs - the server appears to hang. The nginx logs show:

192.168.32.2 - - [15/Jul/2020:21:03:33 +0000] "POST /authn/myConjurAccount/host%2FBotApp%2FmyDemoApp/authenticate HTTP/1.1" 200 644 "-" "curl/7.55.0"
192.168.32.2 - - [15/Jul/2020:21:03:35 +0000] "GET /secrets/myConjurAccount/variable/BotApp%2FsecretVar HTTP/1.1" 401 27 "-" "curl/7.55.0"

I am going to try again going from 1.7.4 to 1.8.1, but if I get the same result I will likely need help from @uCatu and team to figure out next steps. I don't think we'll include post-1.8 releases in the suite release until this is resolved.

ETA: I have confirmed that the same problem occurs:

when upgrading from 1.7.4 to 1.8.1 following the standard instructions + the custom documented step for 1.8.0
when upgrading from 1.7.4 to 1.8.0 following the standard instructions

uCatu

WIP please

uCatu · 2020-07-16T08:04:13Z

UPGRADING.md

+
+1. View Docker containers and verify all are healthy, up and running:
+   ```
+   docker ps -a


Suggest using docker-compose as we assum user also use. And in case user has additional container that are not related to conjur they will not appear in output
docker-compose ps -a

uCatu · 2020-07-16T08:05:39Z

UPGRADING.md

+using Docker Compose. These steps assume you have defined your Conjur image in
+a service named `conjur`, and that you have access to the Conjur data key
+that was used when you originally deployed your Conjur server.
+1. In your terminal window, set the `CONJUR_DATA_KEY` environment variable:


Suggested change

1. In your terminal window, set the `CONJUR_DATA_KEY` environment variable:

1. In local terminal session, set `CONJUR_DATA_KEY` environment variable:

uCatu · 2020-07-16T08:13:00Z

UPGRADING.md

+
+The following steps describe a standard upgrade of the Conjur server, when deployed
+using Docker Compose. These steps assume you have defined your Conjur image in
+a service named `conjur`, and that you have access to the Conjur data key


The mention of Conjur data key is much more important than naming of conjur service
Can we put the spotlight on it? It can be confusing with API KEY I think we need to give here a link (maybe from here or here ) to where we suggsted to store it, that it's a Base64-encoded 256 bit data encrytion key etc, without it - there is no upgrade

uCatu · 2020-07-16T08:14:02Z

UPGRADING.md

+that was used when you originally deployed your Conjur server.
+1. In your terminal window, set the `CONJUR_DATA_KEY` environment variable:
+   ```
+   export CONJUR_DATA_KEY={your Conjur data key}


Suggested change

export CONJUR_DATA_KEY={your Conjur data key}

export CONJUR_DATA_KEY=<Conjur data key content>

uCatu · 2020-07-16T08:14:48Z

UPGRADING.md

+   ```
+
+   It may also be useful to check the logs of the Conjur
+   container to ensure that Puma started successfully, which can be done by


Don't know if we want to talk about Puma here, adding comlpexitiy

uCatu · 2020-07-16T08:20:07Z

UPGRADING.md

+### Troubleshooting
+
+If you run through the steps above _without_ setting the `CONJUR_DATA_KEY`
+environment variable first, you will be able to complete the steps successfully


Can we change "succesfully" to "without visible/explicit error message"

uCatu · 2020-07-16T08:20:47Z

UPGRADING.md

+environment variable first, you will be able to complete the steps successfully
+but the logs of the new Conjur container will show an error like:
+```
+$ docker logs conjur_server


If we chose to use docer-compose please switch this example to the one above

uCatu · 2020-07-16T08:21:00Z

UPGRADING.md

+`conjur_server` in the example command above is defined as the `container_name`
+of the `conjur` service in the `docker-compose.yml`.


Redundent in any case

UPGRADING.md

uCatu · 2020-07-16T08:23:04Z

UPGRADING.md

+newer version.**
+
+Starting in version 1.8.0, the hashing algorithm used to fingerprint and identify
+encryption keys was changed from MD5 to SHA256. This fingerprint is stored in


We want to add
"from MD5 to a more secure SHA256" ?

uCatu

WIP please

uCatu · 2020-07-16T08:33:21Z

UPGRADING.md

+the process again. This time when you check the logs of the Conjur server
+container you should see the Puma service starting as expected:
+```
+$ docker logs conjur_server


Maybe this command should be better? encapsulate with CLI
docker-compose exec conjur conjurctl wait

I'm glad you suggested that! I use that command myself and was trying to find a way to work it in - I hesitated, because I'm not sure it works well enough when the server fails. I'll play around with it and see how it goes.

izgeri · 2020-07-16T15:33:52Z

@uCatu thanks for your feedback, and I'll try to work in your changes.

IMPORTANTLY the custom upgrade instructions for v1.8.0 don't work as currently written. do you have any more info on this?

uCatu · 2020-07-16T15:58:28Z

@uCatu thanks for your feedback, and I'll try to work in your changes.

IMPORTANTLY the custom upgrade instructions for v1.8.0 don't work as currently written. do you have any more info on this?

@izgeri
Yes I'm on it will take me 30 minutes to figure out if we can use slosilo task - as it never been tested. The original approach was to change fingerprint manual on PG via psql - just wanted to touch base on the whole tutorial since it's the first time I'm reading it, thats why I commented on other issues

Available on Slack for quick response :)

uCatu · 2020-07-16T16:53:49Z

@uCatu thanks for your feedback, and I'll try to work in your changes.
IMPORTANTLY the custom upgrade instructions for v1.8.0 don't work as currently written. do you have any more info on this?

@izgeri
Yes I'm on it will take me 30 minutes to figure out if we can use slosilo task - as it never been tested. The original approach was to change fingerprint manual on PG via psql - just wanted to touch base on the whole tutorial since it's the first time I'm reading it, thats why I commented on other issues

Available on Slack for quick response :)

Revisiting Slosilo migrate task:
https://github.com/cyberark/slosilo/blob/4c4c6d2beac698b8958acab5627d926fe240c6fb/lib/slosilo/adapters/sequel_adapter.rb#L52

tells us that in order to get fingerprint value, fingrperint column must be deleted or not present
as we are talking about upgrade the latter is not relevent

That means that we can stil utilize that task by still needs to (was able to create this oneliner):
docker-compose exec -u postgres database psql -c "ALTER TABLE slosilo_keystore DROP COLUMN fingerprint"

and than run:
docker-compose exec conjur bundle exec rake slosilo:migrate

I tested it, looks fine can you verify please?

izgeri

I deployed v1.5.0 using the quick start instructions

Then I built Conjur locally and changed the docker-compose to read conjur:1.8.1-14274915 for the image. I followed the upgrade instructions and it worked beautifully! Very nice work @h-artzi 👏

I did make one suggestion to change a section title. But other than that, I think this PR is good to go.

UPGRADING.md

izgeri · 2020-09-03T19:16:52Z

@h-artzi it looks like it also needs a rebase before I can approve

izgeri

Great work to everyone involved! Thanks @hilagross for kicking this off, @uCatu for drafting an initial process, and @h-artzi for getting it over the finish line! It feels great to have this added to our project :)

@uCatu

@uCatu I believe @h-artzi addressed all of your requested changes, with one optional request that seems ok to skip. If you have any additional requests, let's work on them in a follow-up issue - this one's been open long enough!

@alexkalish

Dismissing your review @alexkalish because it would appear all of your changes have been addressed. Thanks for helping make this a better document!!

README.md

codeclimate · 2020-09-03T20:26:40Z

Code Climate has analyzed commit 9a661bc and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 86.8% (0.0% change).

View more on Code Climate.

alexkalish · 2020-09-04T13:19:24Z

Wohoo!!

hilagross requested a review from alexkalish June 10, 2020 05:45

hilagross self-assigned this Jun 10, 2020

hilagross mentioned this pull request Jun 10, 2020

Conjur upgrade instructions for FIPS compliance #1584

Closed

izgeri suggested changes Jun 10, 2020

View reviewed changes

alexkalish previously requested changes Jun 19, 2020

View reviewed changes

izgeri force-pushed the hilagross-patch-1 branch from bdceab6 to d5bedcd Compare July 15, 2020 21:06

hilagross requested a review from a team as a code owner July 15, 2020 21:06

izgeri force-pushed the hilagross-patch-1 branch from d5bedcd to fa57c08 Compare July 15, 2020 21:06

uCatu suggested changes Jul 16, 2020

View reviewed changes

uCatu previously requested changes Jul 16, 2020

View reviewed changes

h-artzi mentioned this pull request Aug 27, 2020

add migration step that recalculates fingerprints #1782

Merged

6 tasks

h-artzi force-pushed the hilagross-patch-1 branch from fa57c08 to e75a5fb Compare August 31, 2020 18:23

h-artzi closed this Aug 31, 2020

h-artzi force-pushed the hilagross-patch-1 branch from e75a5fb to a9f157e Compare August 31, 2020 20:59

h-artzi reopened this Aug 31, 2020

h-artzi requested a review from izgeri August 31, 2020 21:00

izgeri suggested changes Sep 3, 2020

View reviewed changes

UPGRADING.md Outdated Show resolved Hide resolved

h-artzi force-pushed the hilagross-patch-1 branch from ca812d9 to 8e313d3 Compare September 3, 2020 19:18

Create UPGRADING.md

026a188

h-artzi force-pushed the hilagross-patch-1 branch from 8e313d3 to 026a188 Compare September 3, 2020 19:19

izgeri previously approved these changes Sep 3, 2020

View reviewed changes

h-artzi dismissed izgeri’s stale review via b3a5d14 September 3, 2020 19:41

add CHANGELOG entry for upgrade instructions

2f696d3

h-artzi force-pushed the hilagross-patch-1 branch from b3a5d14 to 2f696d3 Compare September 3, 2020 19:48

izgeri reviewed Sep 3, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

izgeri reviewed Sep 3, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

izgeri previously approved these changes Sep 3, 2020

View reviewed changes

update README to include upgrade instructions

9a661bc

h-artzi dismissed izgeri’s stale review via 9a661bc September 3, 2020 20:09

h-artzi force-pushed the hilagross-patch-1 branch from 79b134f to 9a661bc Compare September 3, 2020 20:09

izgeri approved these changes Sep 3, 2020

View reviewed changes

izgeri assigned h-artzi and uCatu Sep 3, 2020

izgeri merged commit 3b7cbe2 into master Sep 3, 2020

izgeri deleted the hilagross-patch-1 branch September 3, 2020 20:33

izgeri mentioned this pull request Sep 3, 2020

Conjur has clear upgrade instructions #1528

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create UPGRADING.md #1607

Create UPGRADING.md #1607

hilagross commented Jun 10, 2020 •

edited by izgeri

Loading

izgeri Jun 10, 2020

hilagross Jun 15, 2020 •

edited

Loading

izgeri Jun 15, 2020

hilagross Jun 16, 2020

izgeri Jun 16, 2020

hilagross Jun 17, 2020

alexkalish Jun 19, 2020

izgeri commented Jun 16, 2020

alexkalish left a comment

alexkalish Jun 19, 2020

InbalZilberman commented Jul 14, 2020

izgeri commented Jul 15, 2020

izgeri commented Jul 15, 2020 •

edited

Loading

uCatu left a comment

uCatu Jul 16, 2020 •

edited

Loading

uCatu Jul 16, 2020

uCatu Jul 16, 2020

uCatu Jul 16, 2020

uCatu Jul 16, 2020

uCatu Jul 16, 2020

uCatu Jul 16, 2020

uCatu Jul 16, 2020

uCatu Jul 16, 2020

uCatu left a comment

uCatu Jul 16, 2020

izgeri Jul 16, 2020

izgeri commented Jul 16, 2020

uCatu commented Jul 16, 2020

uCatu commented Jul 16, 2020

izgeri left a comment

izgeri commented Sep 3, 2020

izgeri left a comment

codeclimate bot commented Sep 3, 2020

alexkalish commented Sep 4, 2020


		## Specific steps

		This step should be done when upgrading from version 1.7.2 and below to a newer version

	1. In your terminal window, set the `CONJUR_DATA_KEY` environment variable:
	1. In local terminal session, set `CONJUR_DATA_KEY` environment variable:

	export CONJUR_DATA_KEY={your Conjur data key}
	export CONJUR_DATA_KEY=<Conjur data key content>

		`conjur_server` in the example command above is defined as the `container_name`
		of the `conjur` service in the `docker-compose.yml`.

Create UPGRADING.md #1607

Create UPGRADING.md #1607

Conversation

hilagross commented Jun 10, 2020 • edited by izgeri Loading

What does this PR do?

What ticket does this PR close?

Checklists

Change log

Test coverage

Documentation

Choose a reason for hiding this comment

hilagross Jun 15, 2020 • edited Loading

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

izgeri commented Jun 16, 2020

alexkalish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InbalZilberman commented Jul 14, 2020

izgeri commented Jul 15, 2020

izgeri commented Jul 15, 2020 • edited Loading

uCatu left a comment

Choose a reason for hiding this comment

uCatu Jul 16, 2020 • edited Loading

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

uCatu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

izgeri commented Jul 16, 2020

uCatu commented Jul 16, 2020

uCatu commented Jul 16, 2020

izgeri left a comment

Choose a reason for hiding this comment

izgeri commented Sep 3, 2020

izgeri left a comment

Choose a reason for hiding this comment

codeclimate bot commented Sep 3, 2020

alexkalish commented Sep 4, 2020

hilagross commented Jun 10, 2020 •

edited by izgeri

Loading

hilagross Jun 15, 2020 •

edited

Loading

izgeri commented Jul 15, 2020 •

edited

Loading

uCatu Jul 16, 2020 •

edited

Loading