* Partially convert EppResourceUtils to SQL
Some of the rest will depend on b/184578521.
The primary conversion in this PR is the change in
NameserverLookupByIpCommand as that is the only place where the removed
EppResourceUtils method was called. We also convert to DualDatabaseTest
the tests of the callers of NLBIC. and use a CriteriaQueryBuilder in the
foreign key index SQL lookup (allowing us to avoid the String.format
call).
* Remove SQL credentials from Keyring
Remove SQL credentials from Keyring. SQL credentials will be managed by
an automated system (go/dr-sql-security) and the keyring is no longer a
suitable place to hold them.
Also stopped loading SQL credentials from they keyring for comparison
with those from the secret manager.
* Convert RefreshDnsOnHostRenameAction to tm
This is not quite complete because it also requires the conversion of a
map-reduce which is in scope for an entirely different work. Tests of the
map-reduce functionality are excluded from the SQL run.
This also requires the following additional fixes:
- Convert Lock to tm, as doing so was necessary to get this action to work.
As Lock is being targeted as DatastoreOnly, we convert all calls in it to
use ofyTm()
- Fix a bug in DualDatabaseTest (the check for an AppEngineExtension field is
wrong, and captures fields of type Object as AppEngineExtension's)
- Introduce another VKey.from() method that creates a VKey from a stringified
Ofy Key.
* Rename VKey.from(String) to fromWebsafeKey
* Throw NoSuchElementE. instead of NPE
* Correctly get the primary database value in PremiumListDualDao
* Remove extra AppEngineExtension
* get rid of ofy call
* Remove extra duration skip in test
* Convert poll-message-related classes to use SQL as well
Two relatively complex parts. The first is that we needed a small
refactor on the AckPollMessagesCommand because we could theoretically be
acking more poll messages than the Datastore transaction size boundary.
This means that the normal flow of "gather the poll messages from the DB
into one collection, then act on it" needs to be changed to a more
functional flow.
The second is that acking the poll message (deleting it in most cases)
reduces the number of remaining poll messages in SQL but not in
Datastore, since in Datastore the deletion does not take effect until
after the transaction is over.
* Fix some low-hanging code quality issue fruits
These include problems such as: use of raw types, unnecessary throw clauses,
unused variables, and more.
* Convert ofy -> tm for two more classes
Convert ofy -> tm for MutatingCommand and DedupeOneTimeBillingEventIdsCommand.
Note that DedupeOneTimeBillingEventIdsCommand will not be needed after
migration, so this conversion is just to remove the ofy uses from the
codebase. We don't update the test (other than to keep it working) and it
wouldn't currently work in SQL.
* Fixed a test broken by this PR
In addition, we move the deleteTestDomain method to DatabaseHelper since
it'll be useful in other places (e.g. RelockDomainActionTest) and remove
the duplicate definition of ResaveEntityAction.PATH.
We also can ignore deletions of non-persisted entities in the JPA
transaction manager.
* Update RegistrarSettingsAction and RegistrarContact to SQL calls
Relevant potentially-unclear changes:
- Making sure the last update time is always correct and up to date in
the auto timestamp object
- Reloading the domain upon return when updating in a new transaction to
make sure that we use the properly-updated last update time (SQL returns
the correct result if retrieved within the same txn but DS does not)
* Convert DomainTAF and DomainFlowUtils to SQL
The only tricky part to this is that the order of entities that we're
saving during the DomainTransferApproveFlow matters -- some entities
have dependencies on others so we need to save the latter first. We
change `entitiesToSave` to be a list to reinforce this.
Various necessary changes included as part of this:
- Make ForeignKeyIndex completely generic. Previously, only the load()
method that took a DateTime as input could use SQL, and the cached flow
was particular to Objectify Keys. Now, the cached flow and the
non-cached flow can use the same (ish) piece of code to load / create
the relevant index objects before filtering or modifying them as
necessary.
- EntityChanges should use VKeys
- FlowUtils should persist entity changes using tm(), however not all
object types are storable in SQL.
- Filling out PollMessage fields with the proper object type when
loading from SQL
- Changing a few tm() calls to ofyTm() calls when using objectify. This
is because creating a read-only transaction in SQL is quite a footgun at
the moment, because it makes the entire transaction you're in (if you
were already in one) a read-only transaction.
* Convert 3 classes from ofy -> tm
Convert SyncGroupMembersAction, SyncRegistrarsSheet and
IcannReportingUploadAction and their test cases to use TransactionManager and
dual-test them so we know they work in jpa.
* Address comments in review
Address review comments and make the entire IcannReportingUploadAction run
transactional.
* reformatted.
* Remove duplicate loadByKey() method
Remove test method added in a recent PR.
* Embed a ZonedDateTime as the UpdateAutoTimestamp in SQL
This means we can get rid of the converter and more importantly, means
that reading the object from SQL does not affect the last-read time (the
test added to UpdateAutoTimestampTest failed prior to the production
code change).
For now we keep both time fields in UpdateAutoTimestamp however
post-migration, we can remove the joda-time field if we wish.
Note: I'm not sure why <now> is the time that we started getting
LazyInitializationExceptions in the LegacyHistoryObject and
ReplayExtension tests but we can solve that by just examining /
initializing the object within the transaction.
* Make TransactionManager.loadAllOf() smart w.r.t the cross-TLD entity group
The loadAllOf() method will now automatically append the cross-TLD entity group
ancestor query as necessary, iff the entity class being loaded is tagged with
the new @IsCrossTld annotation.
* Add tests
* Add SQL wipeout action in QA
Added the WipeOutSqlAction that deletes all data in Cloud SQL.
Wipe out is restricted to the QA environment, which will get production
data during migration testing.
Also added a cron job that invokes wipeout on every saturday morning.
This is part of the privacy requirments for using production data in QA.
Tested in QA.
* Add some load convenience methods to DatabaseHelper
These can only be called by test code, and they automatically wrap the load
in a transaction if one isn't already specified (for convenience).
In production code we don't want to be able to use these, as we have to be
more thoughtful about transactions in production code (e.g. make sure that
we aren't loading and then saving a resource in separate transactions in a
way that makes it prone to contention errors).
* Add Gradle tasks to stage BEAM pipelines
Add a Gracle task to stage flex-template based pipelines for alpha and
crash environments.
This is a follow up to go/r3pr/1028, which is also under review.
Add the actual release tag and beam staging project id to the config
file. This allows the Nomulus server to find the right version of the
BEAM pipelines to launch.
* Disallow admin triggering of internal endpoints
Stop simply relying on the presence of the X-AppEngine-QueueName as an
indicator that an endpoint has been triggered internally, as this allows
admins to trigger a remote execution vulnerability.
We now supplement this check by ensuring that there is no authenticated user.
Since only an admin user can set these headers, this means that the header
must have been set by an internal request.
Tested:
In addition to the new unit test, verified on Crash that:
- Internal requests are still getting authenticated via the internal auth
mechanism.
- Admin requests with the X-AppEngine-QueueName header are rejected as
"unauthorized."
* Reformatted.
This has the same issue as the domain-search action where the database
ordering is not consistent between Objectify and SQL -- as a result,
there is one test that we have to duplicate in order to account for the
two sort orders.
In addition, there isn't a way to query @Convert-ed fields in Postgres
via the standard Hibernate / JPA query language, meaning we have to use
a raw Postgres query for that.
* Add a jpaTm().query(...) convenience method
This replaces the more ungainly jpaTm().getEntityManager().createQuery(...).
Note that this is in JpaTransactionManager, not the parent TransactionManager,
because this is not an operation that Datastore can support. Once we finish
migrating away from Datastore this won't matter anyway because
JpaTransactionManager will be merged into TransactionManager and then deleted.
In the process of writing this PR I discovered several other methods available
on the EntityManager that may merit their own convenience methods if we start
using them enough. The more commonly used ones will be addressed in subsequent
PRs. They are:
jpaTm().getEntityManager().getMetamodel().entity(...).getName()
jpaTm().getEntityManager().getCriteriaBuilder().createQuery(...)
jpaTm().getEntityManager().createNativeQuery(...)
jpaTm().getEntityManager().find(...)
This PR also addresses some existing callsites that were calling
getEntityManager() rather than using extant convenience methods, such as
jpa().insert(...).
* Add replay to remaining (non-trivial) flow tests
Convert all remaining flow tests to do replay/compare testing. In the course
of this:
- Move the class specific SetClock extension into its own place.
- Fix another "cyclic" foreign key (there may be another solution in this case
because HostHistory is actually different from HistoryEntry, but that would
require changing the way we establish priority since HostHistory is not
distinguished from HistoryEntry in the current methodology)
* Attempt to fix flakey deleteExpiredDomain test
Though hard to reproduce locally, the test_deletesThreeDomainsInOneRun
test has failed multiple times on Kokoro. The root cause may be the
non-transactional query executed by the Action object, which was by
design. Observing that the other test never fails, this PR follows its behavior
and adds a transactional query before invoking the action.
The dual DAO takes care of switching between databases, comparing the
results of one to the results of the other, and caching the result. All
calls to ClaimsList retrieval or storing should use the
dual-database-DAO.
Previously, calls to comparing the lists were somewhat scattered
throughout the codebase. Now, there is one class for retrieval and
comparison (the dual DAO), one class for retrieval from SQL (the SQL
DAO), and one class for retrieval from Datastore (ClaimsListShard
itself, though the retrieval could be moved in to a separate DAO if we
wished).
In addition, we rename the ClaimsListDao to ClaimsListSqlDao
* Update creation script for schema_deployer
Move the create user command for schema_deployer before the
initialization of roles. As the owner of all schema objects, it needs to
be present before grant statements are executed.
Also fixed a bug in credential printing, which fails when the password
contains '%'.
This allows us to get rid of the DAO as well as the sanity-checking
methods since we can be reasonably sure that the fields will be the
same. Future PRs will add conversions from ofy() to tm() calls that will
make sure that we get the same proper data in both Datastore and SQL
* Convert more flow tests to replay/compare
Add the replay extension to another batch of flow tests. In the course of
this:
- Refactor out domain deletion code into DatabaseHelper so that it can be used
from multiple tests.
- Make null handling uniform for contact phone numbers.
* Convert postLoad method to onLoad.
* Remove "Test" import missed during rebase
* Deal with persistence of billing cancellations
Deal with the persistence of billing cancellations, which were added in the
master branch since before this PR was initially sent for review.
* Adding forgotten flyway file
* Removed debug variable
* Add schema_deployer SQL user to SecretManager
Add the 'schema_deployer' user to the SecretManager so that its
credential can be set up. The schema deployment process will use this
user instead of the 'postgres' user.
Changed the output of the get_sql_credential command for the schema
deployment process.
Added a sql script that documents the privileges granted to
'schema_deployer'.
* Clear autorenew end time when a domain is restored
This allows us to still see in the database which now-deleted domains had
reached expiration, while correctly not re-deleting the domain immediately if
the registrar pays to explicitly restore the domain.
This also resolves some TODOs around data migration for this field on domain so
that it's not null, as said migration has already been completed.
* Remove grace period ID @OnLoads now that migration is complete
I verified in BigQuery that all grace period IDs are now allocated (as expected
given that the re-save all EPP resource mapreduce has been run several times
since this migration started last year). The query I used for verification is:
SELECT fullyQualifiedDomainName, gp, ot
FROM `domain-registry.latest_datastore_export.DomainBase`
JOIN UNNEST(gracePeriods.billingEventRecurring) AS gp
JOIN UNNEST(gracePeriods.billingEventOneTime) AS ot
WHERE gp.id IS NULL or ot.id IS NULL
BUG=169873747
* Add daily cron entries to for DeleteExpiredDomainsAction
This also requires setting this action to GET instead of POST, as GAE cron makes
GET requests.
* Use shared jar to stage BEAM pipeline if possible
Allow multiple BEAM pipelines with the same classes and dependencies to
share one Uber jar.
Added metadata for BulkDeleteDatastorePipeline.
Updated shell and Cloud Build scripts to stage all pipelines in one
step.
* Add comments to Cloud SQL configs
I believe the similarity in trace to https://github.com/brettwooldridge/HikariCP/issues/1212
is misleading.
The real cause of the exceptions may be that we ran out of connections. At the
time, the production Cloud SQL server could handle 500 connections at the
maximum. That number was within reach of a busy Nomulus server.
The maximum connection in production has been increased to 1000. We
haven't encountered this issue for a long time. All connection problems
are due to Cloud SQL maintenance or other GCP related issues.
This issue is tracked by b/154720215, which is being closed with this
PR.
* Convert DomainTransferRequestFlow to tm() calls
Besides the standard ofy-to-tm conversions this includes storing the
billing event cancellation VKey in the DomainTransferData object so that
we know to handle it on process / cancellation.
